Self Hosting: Docker Compose setup and flow #640

Merged
Radu-C-Martin merged 4 commits from main into main 2024-05-07 06:52:14 +08:00
Radu-C-Martin commented 2024-04-18 05:25:15 +08:00 (Migrated from github.com)

My attempt at implementing Issue #627.

I'm for sure missing some stuff, so please don't hesitate to mention it.

The force_ssl environment configs are based on this discussion.

My attempt at implementing Issue #627. I'm for sure missing some stuff, so please don't hesitate to mention it. The `force_ssl` environment configs are based on [this](https://github.com/maybe-finance/maybe/issues/308) discussion.
zachgoll commented 2024-04-18 09:21:33 +08:00 (Migrated from github.com)

@Radu-C-Martin nice start here!

I think the goal for this issue will be to merge an "all in one" solution that is well tested and validated across the entire self-hosting flow, which will include:

  • Successful runs of the Github action + publishing to GHCR (I'm guessing a forked repo is probably the best way to fully test, but open for suggestions)
  • Some documentation that walks a self-hoster start to finish using this solution to successfully deploy and access their Maybe instance

Any thoughts on how we can test and validate amongst the community? I'd imagine we'll have quite a few different types of setups to run through.

@Radu-C-Martin nice start here! I think the goal for this issue will be to merge an "all in one" solution that is well tested and validated across the entire self-hosting flow, which will include: - Successful runs of the Github action + publishing to GHCR (I'm guessing a forked repo is probably the best way to fully test, but open for suggestions) - Some documentation that walks a self-hoster start to finish using this solution to successfully deploy and access their Maybe instance - We'll need a `docker.md` document here - https://github.com/maybe-finance/maybe/tree/main/docs/self-hosting Any thoughts on how we can test and validate amongst the community? I'd imagine we'll have quite a few different types of setups to run through.
zachgoll commented 2024-04-19 02:40:04 +08:00 (Migrated from github.com)

I'm going to mark this pull request as a draft and would encourage any other self-hosters to collaborate on this branch to get this to completion. Would love to hear more thoughts around:

  • The best way to test and collaborate on a flow like this
  • What self-hosted Docker flows do we need to prioritize and support? (i.e. fresh VPS setup, integrating with HomeLab setups, etc.)
  • What is our long-term method for ensuring this flow stays up-to-date as the app evolves
I'm going to mark this pull request as a draft and would encourage any other self-hosters to collaborate on this branch to get this to completion. Would love to hear more thoughts around: - The best way to test and collaborate on a flow like this - What self-hosted Docker flows do we need to prioritize and support? (i.e. fresh VPS setup, integrating with HomeLab setups, etc.) - What is our long-term method for ensuring this flow stays up-to-date as the app evolves
Radu-C-Martin commented 2024-04-24 03:35:27 +08:00 (Migrated from github.com)

I played around with this some more.

I adjusted the deployment workflow to tag not only the complete version, but also a major.minor tag and a major-only tag (this probably would only make sense if breaking changes only happen in major versions?).

I also added a step to prune the untagged images when deploying, since creating a new image for every commit will generate quite a few images :)

Additionally, I updated the documentation a bit to reflect the docker deployment. From commercial VPS I could only test Linode, but it works well there.

I left the references to my images in the docker-compose file for now, so that people can play around with that and suggest changes.

One thing to note is that the latest tag follows the last image that commit that was tagged, so for example if you first tag a v.2.1.2, and then a fix for v.1.5.6 or something, the latest tag will be on the v.1.5.6. But in that case we would probably need more logic in the workflow anyway, so tell me if that's something you want me to consider :)

I played around with this some more. I adjusted the deployment workflow to tag not only the complete version, but also a major.minor tag and a major-only tag (this probably would only make sense if breaking changes only happen in major versions?). I also added a step to prune the untagged images when deploying, since creating a new image for every commit will generate quite a few images :) Additionally, I updated the documentation a bit to reflect the docker deployment. From commercial VPS I could only test Linode, but it works well there. I left the references to my images in the docker-compose file for now, so that people can play around with that and suggest changes. One thing to note is that the `latest` tag follows the last image that commit that was tagged, so for example if you first tag a `v.2.1.2`, and then a fix for `v.1.5.6` or something, the latest tag will be on the `v.1.5.6`. But in that case we would probably need more logic in the workflow anyway, so tell me if that's something you want me to consider :)
zachgoll commented 2024-04-24 20:07:57 +08:00 (Migrated from github.com)

Coming along nicely! Would love to hear from additional self-hosters on their own use-cases and review of this flow.

Coming along nicely! Would love to hear from additional self-hosters on their own use-cases and review of this flow.
DorianMazur (Migrated from github.com) reviewed 2024-04-25 03:14:50 +08:00
DorianMazur (Migrated from github.com) commented 2024-04-25 03:14:50 +08:00

One question, why do we need redis here?

One question, why do we need redis here?
DorianMazur (Migrated from github.com) reviewed 2024-04-25 03:51:18 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
DorianMazur (Migrated from github.com) commented 2024-04-25 03:51:18 +08:00

It would be nice to build docker image for arm64

It would be nice to build docker image for arm64
Radu-C-Martin (Migrated from github.com) reviewed 2024-04-25 03:54:39 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
Radu-C-Martin (Migrated from github.com) commented 2024-04-25 03:54:39 +08:00

That's probably more a question to the core maintainers since that could bring some additional overhead :D

That's probably more a question to the core maintainers since that could bring some additional overhead :D
Radu-C-Martin (Migrated from github.com) reviewed 2024-04-25 03:55:49 +08:00
Radu-C-Martin (Migrated from github.com) commented 2024-04-25 03:55:49 +08:00

I based the docker-compose on the devcontainer one, which has redis as well, but as far as I can see it's not actually used?

I based the docker-compose on the devcontainer one, which has redis as well, but as far as I can see it's not actually used?
zachgoll (Migrated from github.com) reviewed 2024-04-25 03:57:23 +08:00
zachgoll (Migrated from github.com) commented 2024-04-25 03:57:23 +08:00

We won't need Redis for this first pass. It is not being used within the app currently.

We won't need Redis for this first pass. It is not being used within the app currently.
zachgoll (Migrated from github.com) reviewed 2024-04-25 04:03:24 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
zachgoll (Migrated from github.com) commented 2024-04-25 04:03:23 +08:00

For this first workflow, I think we should focus on keeping things as simple and opinionated as possible. Once we have a workflow merged and in place, we can introduce subsequent PRs to enhance the flow with additional architecture targets, etc.

For this first workflow, I think we should focus on keeping things as simple and opinionated as possible. Once we have a workflow merged and in place, we can introduce subsequent PRs to enhance the flow with additional architecture targets, etc.
zachgoll (Migrated from github.com) reviewed 2024-04-25 04:05:50 +08:00
zachgoll (Migrated from github.com) commented 2024-04-25 04:05:50 +08:00

The RUN_DB_MIGRATIONS_IN_BUILD_STEP is specific to the Render deployments so I believe it can safely be removed here.

The `RUN_DB_MIGRATIONS_IN_BUILD_STEP` is specific to the Render deployments so I believe it can safely be removed here.
DorianMazur (Migrated from github.com) reviewed 2024-04-25 04:45:14 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
DorianMazur (Migrated from github.com) commented 2024-04-25 04:45:14 +08:00
It's working perfectly fine on arm -> https://github.com/DorianMazur/maybe/actions/runs/8822599388/job/24221266397
zachgoll (Migrated from github.com) reviewed 2024-04-25 05:09:39 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
zachgoll (Migrated from github.com) commented 2024-04-25 05:09:39 +08:00

@DorianMazur couple questions:

  1. What device do you need this architecture for?
  2. Any way we could reduce the build time on this?
@DorianMazur couple questions: 1. What device do you need this architecture for? 2. Any way we could reduce the build time on this?
DorianMazur (Migrated from github.com) reviewed 2024-04-25 05:22:41 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
DorianMazur (Migrated from github.com) commented 2024-04-25 05:22:41 +08:00

@zachgoll Maybe we could reduce build time by using macos-latest runners (arm64 architecture). Because currently in my workflow I am using ubuntu-latest that is x86_64.

I am working on arm64, I have multiple homeLab servers (each of them is using 5W of power) on arm64 and my Macbook is also arm64. That's why I need this, probably not only me :)

@zachgoll Maybe we could reduce build time by using `macos-latest` runners (arm64 architecture). Because currently in my workflow I am using ubuntu-latest that is x86_64. I am working on arm64, I have multiple homeLab servers (each of them is using 5W of power) on arm64 and my Macbook is also arm64. That's why I need this, probably not only me :)
DorianMazur commented 2024-04-25 05:23:11 +08:00 (Migrated from github.com)

I just tested this docker compose and it's not working for me :/

ActiveRecord::ConnectionNotEstablished: connection to server at "172.29.0.2", port 5432 failed: fe_sendauth: no password supplied (ActiveRecord::ConnectionNotEstablished)
Caused by:
PG::ConnectionBad: connection to server at "172.29.0.2", port 5432 failed: fe_sendauth: no password supplied (PG::ConnectionBad)
Tasks: TOP => db:prepare

EDIT:
It's working now, I removed special characters from password. It seems that they are not supported.

~~I just tested this docker compose and it's not working for me :/~~ ``` ActiveRecord::ConnectionNotEstablished: connection to server at "172.29.0.2", port 5432 failed: fe_sendauth: no password supplied (ActiveRecord::ConnectionNotEstablished) Caused by: PG::ConnectionBad: connection to server at "172.29.0.2", port 5432 failed: fe_sendauth: no password supplied (PG::ConnectionBad) Tasks: TOP => db:prepare ``` EDIT: It's working now, I removed special characters from password. It seems that they are not supported.
zachgoll (Migrated from github.com) reviewed 2024-04-25 05:32:34 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
zachgoll (Migrated from github.com) commented 2024-04-25 05:32:34 +08:00

Gotcha, had forgotten about the M1 chip and Docker compatibility. I'm guessing we'll have a decent number of self-hosters running this on a Mac, so probably worth supporting.

That said, I think we either need to get that build time significantly reduced or we could change the frequency in which we're running these builds.

I was originally thinking we'd build and publish on every commit to main, but it may be more practical to only build and publish for each new release (which we're eventually going to hit a weekly/bi-weekly cycle on).

Gotcha, had forgotten about the M1 chip and Docker compatibility. I'm guessing we'll have a decent number of self-hosters running this on a Mac, so probably worth supporting. That said, I think we either need to get that build time significantly reduced _or_ we could change the frequency in which we're running these builds. I was originally thinking we'd build and publish on every commit to `main`, but it may be more practical to only build and publish for each new release (which we're eventually going to hit a weekly/bi-weekly cycle on).
DorianMazur (Migrated from github.com) reviewed 2024-04-25 05:35:48 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
DorianMazur (Migrated from github.com) commented 2024-04-25 05:35:48 +08:00

@zachgoll Out of curiosity, why are you worried about build time on public repo? Github actions are free on public repos :D
Anyway, I can reduce build time by using macos runners. I can create PR in future or maybe add some commits to this one.

@zachgoll Out of curiosity, why are you worried about build time on public repo? Github actions are free on public repos :D Anyway, I can reduce build time by using macos runners. I can create PR in future or maybe add some commits to this one.
zachgoll (Migrated from github.com) reviewed 2024-04-25 07:52:02 +08:00
@@ -0,0 +97,4 @@
type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
- name: Build and push Docker image
zachgoll (Migrated from github.com) commented 2024-04-25 07:52:02 +08:00

Not worried about cost—mostly want to be conscious of the overall feedback loop of making modifications to this flow in the future.

If we add this architecture, looks like we can add runs-on: ${{ matrix.os }} syntax (reference) to speed things up.

Not worried about cost—mostly want to be conscious of the overall feedback loop of making modifications to this flow in the future. If we add this architecture, looks like we can add `runs-on: ${{ matrix.os }}` syntax ([reference](https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#example-using-a-multi-dimension-matrix)) to speed things up.
gantoine (Migrated from github.com) reviewed 2024-04-26 09:37:08 +08:00
gantoine (Migrated from github.com) left a comment

looks good, can't wait to see this merged! just a couples questions/comments

looks good, can't wait to see this merged! just a couples questions/comments
gantoine (Migrated from github.com) commented 2024-04-26 09:30:16 +08:00
Used by rails directly https://api.rubyonrails.org/v7.1.3.2/classes/Rails/Application.html#method-i-secret_key_base
gantoine (Migrated from github.com) commented 2024-04-26 09:32:49 +08:00

i assume this will change to whoever owns the image?

i assume this will change to whoever owns the image?
gantoine (Migrated from github.com) commented 2024-04-26 09:33:27 +08:00

unless you're expecting devs/contributors to use this file to run it locally, i'd rename it to something like docker-compose.example.yml

unless you're expecting devs/contributors to use this file to run it locally, i'd rename it to something like `docker-compose.example.yml`
gantoine (Migrated from github.com) commented 2024-04-26 09:35:42 +08:00

this might be a little too prescriptive, though anyone who know's what they're doing will likely just copy the contents on docker-compose.yml and adapt it

this might be a little _too_ prescriptive, though anyone who know's what they're doing will likely just copy the contents on docker-compose.yml and adapt it
zachgoll (Migrated from github.com) reviewed 2024-04-26 20:07:09 +08:00
zachgoll (Migrated from github.com) commented 2024-04-26 20:07:09 +08:00

That's a good point—we will not be supporting a dev-optimized compose file since that is already done in .devcontainer. It should be made clear that this should not be used for development.

We should also throw a noticeable comment at the top of the file with some links to the documentation on how to get it running for self-hosting

That's a good point—we will _not_ be supporting a dev-optimized compose file since that is already done in `.devcontainer`. It should be made clear that this should not be used for development. We should also throw a noticeable comment at the top of the file with some links to the documentation on how to get it running for self-hosting
zachgoll (Migrated from github.com) reviewed 2024-04-26 20:15:51 +08:00
zachgoll (Migrated from github.com) commented 2024-04-26 20:15:50 +08:00

For some additional context, this was originally introduced as an alternative to using the credentials:edit for "one-click" Render deploys. My thinking was that generating this key and saving it in an env var would be easier for self-hosters than going through the process of securing config/master.key.

This may or may not be the best way to handle this Docker self hosting flow. Suggestions around this are welcome!

36e8b14486/render.yaml (L29-L34)

For some additional context, this was originally introduced as an _alternative to_ using the `credentials:edit` for "one-click" Render deploys. My thinking was that generating this key and saving it in an env var would be _easier_ for self-hosters than going through the process of securing `config/master.key`. This may or may not be the best way to handle this Docker self hosting flow. Suggestions around this are welcome! https://github.com/maybe-finance/maybe/blob/36e8b144868c9b68b23d61c30ac9a04e29f23318/render.yaml#L29-L34
zachgoll (Migrated from github.com) reviewed 2024-04-26 20:29:22 +08:00
zachgoll (Migrated from github.com) commented 2024-04-26 20:29:22 +08:00

Generally, self-hosters who are on the more "advanced" side will skip guides like this altogether, so I don't think we need to worry about confusion on that end.

IMO, the goal for these instructions should be prescriptive enough so that someone with minimal experience with self-hosting and Docker can successfully get the project running on a fresh VPS.

This is where the One-liner script I was suggesting could come in handy. Steps like "Create a new folder", "Copy docker compose file", "copy .env.example file" could all be great things to throw in a script like that. That way, we could simplify these instructions to something like:

1. Run the `./setup.sh` script
2. Edit `.env`
3. Run `docker-compose up -d` 
Generally, self-hosters who are on the more "advanced" side will skip guides like this altogether, so I don't think we need to worry about confusion on that end. IMO, the goal for these instructions should be prescriptive enough so that someone with minimal experience with self-hosting and Docker can successfully get the project running on a fresh VPS. This is where the [One-liner script](https://github.com/maybe-finance/maybe/issues/627#issuecomment-2060115748) I was suggesting could come in handy. Steps like "Create a new folder", "Copy docker compose file", "copy .env.example file" could all be great things to throw in a script like that. That way, we could simplify these instructions to something like: ``` 1. Run the `./setup.sh` script 2. Edit `.env` 3. Run `docker-compose up -d` ```
zachgoll (Migrated from github.com) reviewed 2024-04-26 20:31:05 +08:00
zachgoll (Migrated from github.com) commented 2024-04-26 20:31:05 +08:00

Unless I'm not thinking about this correctly, we'd set this to Maybe's latest published image.

@Radu-C-Martin I'm guessing this was just here so you could test it on your fork correct?

Unless I'm not thinking about this correctly, we'd set this to Maybe's latest published image. @Radu-C-Martin I'm guessing this was just here so you could test it on your fork correct?
Radu-C-Martin (Migrated from github.com) reviewed 2024-04-26 20:34:16 +08:00
Radu-C-Martin (Migrated from github.com) commented 2024-04-26 20:34:16 +08:00

yes, originally I even pre-setup it with the maybe-finance image name, but then you said you'd like people to test it first, which is a fair point 🤷

yes, originally I even pre-setup it with the maybe-finance image name, but then you said you'd like people to test it first, which is a fair point 🤷
gantoine (Migrated from github.com) reviewed 2024-04-26 23:45:22 +08:00
gantoine (Migrated from github.com) commented 2024-04-26 23:45:22 +08:00

My thinking was that generating this key and saving it in an env var would be easier for self-hosters

Yes IMO it absolutely is 👍🏼

> My thinking was that generating this key and saving it in an env var would be easier for self-hosters Yes IMO it absolutely is 👍🏼
gantoine (Migrated from github.com) reviewed 2024-04-26 23:49:20 +08:00
gantoine (Migrated from github.com) commented 2024-04-26 23:49:19 +08:00

i'm a big fan of one-liner scripts (over prescriptive setup instructions) for those less savvy users who will just copy the instructions anyways. makes it less likely they'll mess something up when copying instructions.

successfully get the project running on a fresh VPS

i wonder what the split between "savvy users who understand docker and compose" and "users running on a cheap vps/rpi" 🤔

i'm a big fan of one-liner scripts (over prescriptive setup instructions) for those less savvy users who will just copy the instructions anyways. makes it less likely they'll mess something up when copying instructions. > successfully get the project running on a fresh VPS i wonder what the split between "savvy users who understand docker and compose" and "users running on a cheap vps/rpi" 🤔
maddox commented 2024-04-30 21:27:39 +08:00 (Migrated from github.com)

Am I missing something, or is there not really a docker image actually available yet?

ERROR: Head "https://ghcr.io/v2/maybe-finance/maybe/manifests/latest": denied
Am I missing something, or is there not really a docker image actually available yet? ERROR: Head "https://ghcr.io/v2/maybe-finance/maybe/manifests/latest": denied
Radu-C-Martin commented 2024-04-30 21:31:21 +08:00 (Migrated from github.com)

Am I missing something, or is there not really a docker image actually available yet?

ERROR: Head "https://ghcr.io/v2/maybe-finance/maybe/manifests/latest": denied

Not available yet, it will be available once this gets merged. This PR also includes the publish workflow 😅

If you want to check if everything works, you could use the ghcr.io/radu-c-martin/maybe image (cf. my fork).

> Am I missing something, or is there not really a docker image actually available yet? > > ``` > ERROR: Head "https://ghcr.io/v2/maybe-finance/maybe/manifests/latest": denied > ``` Not available yet, it will be available once this gets merged. This PR also includes the publish workflow 😅 If you want to check if everything works, you could use the `ghcr.io/radu-c-martin/maybe` image (cf. my fork).
maddox commented 2024-04-30 21:37:26 +08:00 (Migrated from github.com)

Gotcha, thanks!

Gotcha, thanks!
zachgoll commented 2024-05-01 00:20:01 +08:00 (Migrated from github.com)

@Radu-C-Martin I'll do some local testing on all of this today/tomorrow and we'll get it merged!

@Radu-C-Martin I'll do some local testing on all of this today/tomorrow and we'll get it merged!
zachgoll (Migrated from github.com) reviewed 2024-05-01 00:56:59 +08:00
@@ -30,6 +30,10 @@ You can find [detailed setup guides for self hosting here](docs/self-hosting.md)
1. Click the button above
zachgoll (Migrated from github.com) commented 2024-05-01 00:28:31 +08:00
To host Maybe with Docker Compose, please follow our [Docker self-hosting guide](docs/self-hosting/docker.md).
```suggestion To host Maybe with Docker Compose, please follow our [Docker self-hosting guide](docs/self-hosting/docker.md). ```
zachgoll (Migrated from github.com) commented 2024-05-01 00:28:40 +08:00
### Docker
```suggestion ### Docker ```
@@ -0,0 +1,35 @@
services:
zachgoll (Migrated from github.com) commented 2024-05-01 00:31:07 +08:00

I think we can remove HOSTING_PLATFORM. The original reason for introducing this variable was in relation to auto-upgrades, but those assumptions are no longer relevant based on the iterations we've gone through here.

I think we can remove `HOSTING_PLATFORM`. The original reason for introducing this variable was in relation to auto-upgrades, but those assumptions are no longer relevant based on the iterations we've gone through here.
zachgoll (Migrated from github.com) commented 2024-05-01 00:35:59 +08:00

Can we consolidate all of this into the self-hosting/docker.md file?

This file (docs/self-hosting.md) could be as short as:

## Docker 

**Estimated cost:** $5-15 per month

Please see the [Docker self-hosting guide](self-hosting/docker.md) for detailed setup instructions.

And then self-hosting/docker.md would provide these sections:

# Self Hosting Maybe with Docker

## Quick Start

[Concise list of commands to run]

## Prerequisites and Setup

[Required deps for fresh VPS]

## Running the app

[Instructions for getting the app started with compose]

## Updating the App

[Explain the flow of pulling updates by tag / commit and updating instance]

## Where should I host?

### Commercial VPS
### One-Click VPS
### Standalone Image

## Troubleshooting 

[We can leave mostly blank for now, but this will be a catch-all]
Can we consolidate all of this into the `self-hosting/docker.md` file? This file (`docs/self-hosting.md`) could be as short as: ```md ## Docker **Estimated cost:** $5-15 per month Please see the [Docker self-hosting guide](self-hosting/docker.md) for detailed setup instructions. ``` And then `self-hosting/docker.md` would provide these sections: ```md # Self Hosting Maybe with Docker ## Quick Start [Concise list of commands to run] ## Prerequisites and Setup [Required deps for fresh VPS] ## Running the app [Instructions for getting the app started with compose] ## Updating the App [Explain the flow of pulling updates by tag / commit and updating instance] ## Where should I host? ### Commercial VPS ### One-Click VPS ### Standalone Image ## Troubleshooting [We can leave mostly blank for now, but this will be a catch-all] ```
zachgoll (Migrated from github.com) reviewed 2024-05-01 02:54:46 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
zachgoll (Migrated from github.com) commented 2024-05-01 02:54:46 +08:00

Could be wrong, but I don't think Github charges anything for storage on GHCR for public repos?

Either way, I think we can remove this final step here for simplicity.

Could be wrong, but I don't think Github charges anything for _storage_ on GHCR for public repos? Either way, I think we can remove this final step here for simplicity.
zachgoll (Migrated from github.com) reviewed 2024-05-01 04:06:57 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
zachgoll (Migrated from github.com) commented 2024-05-01 04:06:57 +08:00

Can we remove these two steps? It looks like the ubuntu-latest runner already has Docker-Buildx 0.14.0 installed:

https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#tools

Can we remove these two steps? It looks like the `ubuntu-latest` runner already has `Docker-Buildx 0.14.0` installed: https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#tools
zachgoll (Migrated from github.com) requested changes 2024-05-01 06:04:54 +08:00
zachgoll (Migrated from github.com) left a comment

@Radu-C-Martin had some time this afternoon to thoroughly test this. I was able to get it all running on a Digital Ocean droplet, so I think the sample compose file looks good.

Just left some comments around config and documentation that I think we should address before merging this in.

@Radu-C-Martin had some time this afternoon to thoroughly test this. I was able to get it all running on a Digital Ocean droplet, so I think the sample compose file looks good. Just left some comments around config and documentation that I think we should address before merging this in.
@@ -0,0 +1,107 @@
name: Publish Docker image
zachgoll (Migrated from github.com) commented 2024-05-01 05:39:45 +08:00

I'm thinking the only two tags that we'll need to support for now are the "latest" and "release" tags. By using default configs for metadata-action, I believe we can simplify to this:

with:
  images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}
  tags: |
    # set latest tag for main branch
    type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}

    # set semver tag for versioned tags
    type=semver,pattern={{version}}
I'm thinking the only two tags that we'll need to support for now are the "latest" and "release" tags. By using default configs for `metadata-action`, I believe we can simplify to this: ```yml with: images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} tags: | # set latest tag for main branch type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }} # set semver tag for versioned tags type=semver,pattern={{version}} ```
@@ -0,0 +1,35 @@
services:
zachgoll (Migrated from github.com) commented 2024-05-01 05:43:47 +08:00

In addition to this, we will need to hide both "auto updates" and "provider settings" in the self hosted settings since these do not apply to an app running via docker compose:

dc024d63b0/app/views/settings/hostings/show.html.erb (L6-L48)

In addition to this, we will need to hide both "auto updates" and "provider settings" in the self hosted settings since these do not apply to an app running via docker compose: https://github.com/maybe-finance/maybe/blob/dc024d63b0b7b03713a26799cc0d33164011cff3/app/views/settings/hostings/show.html.erb#L6-L48
@@ -0,0 +7,4 @@
restart: unless-stopped
env_file:
- .env
environment:
zachgoll (Migrated from github.com) commented 2024-05-01 05:48:09 +08:00

We should probably add RAILS_ENV=production here too.

We should probably add `RAILS_ENV=production` here too.
Radu-C-Martin (Migrated from github.com) reviewed 2024-05-01 17:16:47 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
Radu-C-Martin (Migrated from github.com) commented 2024-05-01 17:16:47 +08:00

In my experience, the latest tag always points to the latest release. Do you plan for the main branch to always be production-ready?

In my experience, the `latest` tag always points to the latest release. Do you plan for the main branch to always be production-ready?
zachgoll (Migrated from github.com) reviewed 2024-05-01 22:40:13 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
zachgoll (Migrated from github.com) commented 2024-05-01 22:40:12 +08:00

In theory (since we're using trunk-based development), yes, main should always be production ready. If it's not, the checks should fail and the Docker image should not be published.

I would expect latest to represent the most recently published image, which in our case, would always be the latest commit.

But there probably is some value in providing an alias for self-hosters who want to use the "latest release". What do you think of using the following tag scheme?

  • latest - the latest published image (i.e. main)
  • stable - the latest published release
  • [release_semver_tag] - the semver tag of the release
  • [commit_sha] - the commit sha of the docker image so self-hoster can lock into a certain version

So something like:

tags:
  # Explicit commit sha and version tags
  type=sha
  type=semver,pattern={{version}}

  # Aliases
  type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }}
  type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }}
In theory (since we're using trunk-based development), yes, `main` should always be production ready. If it's not, the checks should fail and the Docker image should not be published. I would expect `latest` to represent the _most recently published image_, which in our case, would always be the latest commit. But there probably is some value in providing an alias for self-hosters who want to use the "latest release". What do you think of using the following tag scheme? - `latest` - the latest published image (i.e. `main`) - `stable` - the latest published release - `[release_semver_tag]` - the semver tag of the release - `[commit_sha]` - the commit sha of the docker image so self-hoster can lock into a certain version So something like: ```yml tags: # Explicit commit sha and version tags type=sha type=semver,pattern={{version}} # Aliases type=raw,value=latest,enable=${{ github.ref == 'refs/heads/main' }} type=raw,value=stable,enable=${{ startsWith(github.ref, 'refs/tags/v') }} ```
Quintasan (Migrated from github.com) reviewed 2024-05-02 23:41:06 +08:00
@@ -38,12 +38,12 @@ Rails.application.configure do
# config.action_cable.url = "wss://example.com/cable"
# config.action_cable.allowed_request_origins = [ "http://example.com", /http:\/\/example.*/ ]
Quintasan (Migrated from github.com) commented 2024-05-02 23:39:51 +08:00
  config.force_ssl  = ENV.include?("FORCE_SSL")

I think the default assumption is that you're using a reverse proxy to terminate incoming HTTPS traffic.

```suggestion config.force_ssl = ENV.include?("FORCE_SSL") ``` I think the default assumption is that you're using a reverse proxy to terminate incoming HTTPS traffic.
Quintasan (Migrated from github.com) commented 2024-05-02 23:40:20 +08:00
  config.assume_ssl = ENV.include?["ASSUME_SSL"]
```suggestion config.assume_ssl = ENV.include?["ASSUME_SSL"] ```
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-02 23:37:01 +08:00

I believe this should be:

      - "127.0.0.1:3000:3000"

so the Rails backend is not exposed to the internet without a reverse proxy in front of it

I believe this should be: ```suggestion - "127.0.0.1:3000:3000" ``` so the Rails backend is not exposed to the internet without a reverse proxy in front of it
gantoine (Migrated from github.com) reviewed 2024-05-02 23:46:15 +08:00
@@ -0,0 +1,35 @@
services:
gantoine (Migrated from github.com) commented 2024-05-02 23:46:15 +08:00

I don't think that's needed, and even then if you want to stop it being exposed to the host (not internet, since that would require port-forwarding) just don't bind the ports and use a reverse proxy container.

I don't think that's needed, and even then if you want to stop it being exposed to the host (not internet, since that would require port-forwarding) just don't bind the ports and use a reverse proxy container.
zachgoll (Migrated from github.com) reviewed 2024-05-02 23:54:27 +08:00
@@ -38,12 +38,12 @@ Rails.application.configure do
# config.action_cable.url = "wss://example.com/cable"
# config.action_cable.allowed_request_origins = [ "http://example.com", /http:\/\/example.*/ ]
zachgoll (Migrated from github.com) commented 2024-05-02 23:54:27 +08:00

We've been back and forth on this one quite a bit, but as I was going back through prior discussions, found this comment, which IMO is the clearest solution offered to this:

https://github.com/maybe-finance/maybe/issues/308#issuecomment-2081522347

config.force_ssl = ActiveModel::Type::Boolean.new.cast(ENV.fetch('RAILS_FORCE_SSL', true))
config.assume_ssl = ActiveModel::Type::Boolean.new.cast(ENV.fetch('RAILS_ASSUME_SSL', true))
We've been back and forth on this one quite a bit, but as I was going back through prior discussions, found this comment, which IMO is the clearest solution offered to this: https://github.com/maybe-finance/maybe/issues/308#issuecomment-2081522347 ```rb config.force_ssl = ActiveModel::Type::Boolean.new.cast(ENV.fetch('RAILS_FORCE_SSL', true)) config.assume_ssl = ActiveModel::Type::Boolean.new.cast(ENV.fetch('RAILS_ASSUME_SSL', true)) ```
zachgoll (Migrated from github.com) reviewed 2024-05-02 23:55:42 +08:00
@@ -38,12 +38,12 @@ Rails.application.configure do
# config.action_cable.url = "wss://example.com/cable"
# config.action_cable.allowed_request_origins = [ "http://example.com", /http:\/\/example.*/ ]
zachgoll (Migrated from github.com) commented 2024-05-02 23:55:42 +08:00

Resolving this so we consolidate into one comment.

Resolving this so we consolidate into one comment.
Quintasan (Migrated from github.com) reviewed 2024-05-02 23:58:10 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-02 23:58:10 +08:00

being exposed to the host (not internet, since that would require port-forwarding)

@gantoine I think your vision of how this works is incorrect, especially if you're talking about servers that are exposed to the internet directly.

3000:3000 is equivalent to 0.0.0.0:3000:3000 which means listen on port 3000 on ALL interfaces so if your server has a network interface that's exposed to the internet (literally any VPS) then it will accept incoming connections on port 3000 from virtually anyone. That's not a safe/sane default.

just don't bind the ports and use a reverse proxy container

That's another assumption that's going to make it harder for anyone interested in self-hosting, since your reverse proxy container will listen on ports 80 and 443, and you will have to put any/all subsequent containers in the same Docker network and configure the reverse proxy serving your Maybe instance to serve your other containers or use some convoluted setup involving Traefik and labeling stuff.

I, for example, have a Caddy on the host doing all the reverse proxying to all containers I have on my server like Immich, Maybe, Nextcloud and a ton of other stuff.

>being exposed to the host (not internet, since that would require port-forwarding) @gantoine I think your vision of how this works is incorrect, especially if you're talking about servers that are exposed to the internet directly. `3000:3000` is equivalent to `0.0.0.0:3000:3000` which means `listen on port 3000 on ALL interfaces` so if your server has a network interface that's exposed to the internet (literally any VPS) then it will accept incoming connections on port 3000 from virtually anyone. That's not a safe/sane default. > just don't bind the ports and use a reverse proxy container That's another assumption that's going to make it harder for anyone interested in self-hosting, since your reverse proxy container will listen on ports 80 and 443, and you will have to put any/all subsequent containers in the same Docker network and configure the reverse proxy serving your Maybe instance to serve your other containers or use some convoluted setup involving Traefik and labeling stuff. I, for example, have a Caddy on the host doing all the reverse proxying to all containers I have on my server like Immich, Maybe, Nextcloud and a ton of other stuff.
zachgoll (Migrated from github.com) reviewed 2024-05-03 00:09:07 +08:00
@@ -0,0 +1,35 @@
services:
zachgoll (Migrated from github.com) commented 2024-05-03 00:09:07 +08:00

Just to confirm—you're optimizing for a self-hosting scenario here where you're running this app on your local machine and never want it exposed to the internet?

In your reverse-proxy setup are you assuming you'd run a container (e.g. Traefik), or would you be setting up something like Nginx on the host machine instead?

Just to confirm—you're optimizing for a self-hosting scenario here where you're running this app on your _local_ machine and _never_ want it exposed to the internet? In your reverse-proxy setup are you assuming you'd run a container (e.g. Traefik), or would you be setting up something like Nginx on the _host_ machine instead?
Quintasan (Migrated from github.com) reviewed 2024-05-03 00:17:55 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-03 00:17:55 +08:00

@zachgoll Me? No, I'm optimizing for any sane self-hosting solution - there's a reverse proxy in front of the Rails application. If we put a reverse proxy in the Docker Compose configuration and the self-hosting user has some other services on the server, then they have two options.

  1. Modify the provided docker-compose.yml so the reverse proxy doesn't listen on ports 80 and 443 and point their preferred reverse proxy to... reverse proxy to our reverse proxy
  2. Dump all their other containerized services into the same Docker network and reconfigure our reverse proxy to work with their services.

Maybe there's some convoluted setup leveraging Treafik, but that's not we are aiming for

@zachgoll Me? No, I'm optimizing for any sane self-hosting solution - there's a reverse proxy in front of the Rails application. If we put a reverse proxy in the Docker Compose configuration and the self-hosting user has some other services on the server, then they have two options. 1. Modify the provided `docker-compose.yml` so the reverse proxy doesn't listen on ports 80 and 443 and point their preferred reverse proxy to... reverse proxy to our reverse proxy 2. Dump all their other containerized services into the same Docker network and reconfigure our reverse proxy to work with their services. Maybe there's some convoluted setup leveraging Treafik, but that's not we are aiming for
Quintasan (Migrated from github.com) reviewed 2024-05-03 00:22:02 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-03 00:22:01 +08:00

What we could do is have an additional docker-compose.override.yml sample which has a reverse proxy configured inside. Then we can point everyone who doesn't particularly care about those details to use alongside the default docker-compose.yml

WDYT?

What we *could* do is have an additional `docker-compose.override.yml` sample which has a reverse proxy configured inside. Then we can point everyone who doesn't particularly care about those details to use alongside the default `docker-compose.yml` WDYT?
gantoine (Migrated from github.com) reviewed 2024-05-03 00:58:44 +08:00
@@ -0,0 +1,35 @@
services:
gantoine (Migrated from github.com) commented 2024-05-03 00:58:44 +08:00

@Quintasan Your comment is perfectly reasonable, it's just that I've never seen an example docker-compose bind the network interface (in like, any self-hosted software I use or have tried).

literally any VPS

I'm not thinking about the VPS case, I'm more-so thinking about the user who's first getting started with self hosting on a small server/old laptop/RPI at home.

you will have to put any/all subsequent containers in the same Docker network and configure the reverse proxy serving your Maybe instance to serve your other containers

That's how I have it setup at home, and it works well pretty well 🤷🏼

you're optimizing for a self-hosting scenario here where you're running this app on your local machine and never want it exposed to the internet?

A machine running in my home, not exposed to the internet, accessible via tailscale or cloudflare tunnel. This is the most common setup for users getting into self-hosting, and the one most recommended by reddit, blog posts and youtube channels (which is likely how they're learning to self-host).

@Quintasan Your comment is perfectly reasonable, it's just that I've never seen an example docker-compose bind the network interface (in like, any self-hosted software I use or have tried). > literally any VPS I'm not thinking about the VPS case, I'm more-so thinking about the user who's first getting started with self hosting on a small server/old laptop/RPI at home. > you will have to put any/all subsequent containers in the same Docker network and configure the reverse proxy serving your Maybe instance to serve your other containers That's how I have it setup at home, and it works well pretty well 🤷🏼 > you're optimizing for a self-hosting scenario here where you're running this app on your local machine and never want it exposed to the internet? A machine running in my home, not exposed to the internet, accessible via tailscale or cloudflare tunnel. This is the most common setup for users getting into self-hosting, and the one most recommended by reddit, blog posts and youtube channels (which is likely how they're learning to self-host).
zachgoll (Migrated from github.com) reviewed 2024-05-03 01:15:39 +08:00
@@ -0,0 +1,35 @@
services:
zachgoll (Migrated from github.com) commented 2024-05-03 01:15:39 +08:00

@Quintasan @gantoine thanks for this additional detail, this is helpful.

While I think both of you are probably stronger than me on the config details here, here's the general direction I was thinking:

  • docker-compose.example.yml - this should represent a setup that intersects the 1) most common and 2) most "beginner friendly" setup. It should nearly be an "out of the box" copy/paste solution, or as close as we can get.
  • docker-compose.[specific-setup].example.yml - we could introduce variations of the default as sample files OR add instructions to our documentation
@Quintasan @gantoine thanks for this additional detail, this is helpful. While I think both of you are probably stronger than me on the config details here, here's the general direction I was thinking: - `docker-compose.example.yml` - this should represent a setup that intersects the 1) most common and 2) most "beginner friendly" setup. It should _nearly_ be an "out of the box" copy/paste solution, or as close as we can get. - `docker-compose.[specific-setup].example.yml` - we could introduce variations of the default as sample files OR add instructions to our documentation
Quintasan (Migrated from github.com) reviewed 2024-05-03 01:33:42 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-03 01:33:42 +08:00

it's just that I've never seen an example docker-compose bind the network interface (in like, any self-hosted software I use or have tried).

@gantoine Fair enough. I'm not really going to say that everyone does it so it's a good idea is ever going to convince me, especially once you realize how much automated traffic scanning is going on nowadays. Exposing a web application directly is usually a bad idea considering that webservers like Puma, Webrick etc. are not well-equipped to serve static assets.

@zachgoll I think we should provide a docker-compose.yml in the repository and a docker-compose.override.yml example in the documentation. By default, Docker Compose uses both docker-compose.yml and docker-compose.override.yml when you invoke docker compose up -d so we can support two scenarios out of the box with no convoluted tinkering required:

  1. Bring your own reverse proxy - in this case the self-hoster has just to use the docker-compose.yml in the repository and configure the reverse proxy on their own
  2. I don't care just let me run this - in this case, we would ask the self-hoster to additionally create docker-compose.override.yml in the repository which would set up the reverse proxy for them, so literally no work would be required. Just copy both files and run docker compose up -d and you're ready to go.

In both scenarios, binding the backend port to the loopback interface (127.0.0.1) only prevents external connections from accessing the backend directly on port 3000.

If someone actually wants to expose the backend directly, then they can use docker-compose.override.yml to override our default binding.

> it's just that I've never seen an example docker-compose bind the network interface (in like, any self-hosted software I use or have tried). @gantoine Fair enough. I'm not really going to say that `everyone does it so it's a good idea` is ever going to convince me, especially once you realize how much automated traffic scanning is going on nowadays. Exposing a web application directly is usually a bad idea considering that webservers like Puma, Webrick etc. are not well-equipped to serve static assets. @zachgoll I think we should provide a `docker-compose.yml` in the repository and a `docker-compose.override.yml` example in the documentation. By default, Docker Compose uses both `docker-compose.yml` and `docker-compose.override.yml` when you invoke `docker compose up -d` so we can support two scenarios out of the box with no convoluted tinkering required: 1. Bring your own reverse proxy - in this case the self-hoster has just to use the `docker-compose.yml` in the repository and configure the reverse proxy on their own 2. `I don't care just let me run this` - in this case, we would ask the self-hoster to additionally create `docker-compose.override.yml` in the repository which would set up the reverse proxy for them, so literally no work would be required. Just copy both files and run `docker compose up -d` and you're ready to go. In both scenarios, binding the backend port to the loopback interface (`127.0.0.1`) only prevents external connections from accessing the backend directly on port 3000. If someone **actually** wants to expose the backend directly, then they can use `docker-compose.override.yml` to override our default binding.
EmeraldPi (Migrated from github.com) reviewed 2024-05-03 01:39:44 +08:00
@@ -0,0 +1,35 @@
services:
EmeraldPi (Migrated from github.com) commented 2024-05-03 01:39:43 +08:00

I’m just a self-hosting bystander here, but I would tend to agree with @gantoine. I’ve never seen Docker compose set ups specifically bind the network interface. And, even on a VPS set up, you would need to specifically allow that port in the firewall; at least that’s how it has been for any fresh box I have installed.

I also think if most people are using a Docker set up, there’s no real need to “dumb it down”. They should already understand the risks with simple networking concepts like ports and how to securely access your apps over the internet. I think the Render deploy option serves that purpose well.

I’m just a self-hosting bystander here, but I would tend to agree with @gantoine. I’ve never seen Docker compose set ups specifically bind the network interface. And, even on a VPS set up, you would need to specifically allow that port in the firewall; at least that’s how it has been for any fresh box I have installed. I also think if most people are using a Docker set up, there’s no real need to “dumb it down”. They should already understand the risks with simple networking concepts like ports and how to securely access your apps over the internet. I think the Render deploy option serves that purpose well.
Quintasan (Migrated from github.com) reviewed 2024-05-03 01:51:52 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-03 01:51:52 +08:00

A very, very simplified example:

# docker-compose.yml
# Ours will be slightly more verbose
services:
  backend:
    image: traefik/whoami
    command:
      - --port=6666
    ports:
      - "127.0.0.1:6666:6666"
# docker-compose.override.yml
services:
  caddy:
    image: caddy:alpine
    command: caddy reverse-proxy --from <domain to serve on> --to backend:6666
    ports:
      - 80:80
      - 443:443

If you docker-compose up -d with those two files, then you end up with a backend container and a reverse proxy for it with automatic HTTPS. We can supply a custom Caddyfile OR use Traefik as the reverse proxy if we need a more complicated setup

A very, very simplified example: ``` # docker-compose.yml # Ours will be slightly more verbose services: backend: image: traefik/whoami command: - --port=6666 ports: - "127.0.0.1:6666:6666" ``` ``` # docker-compose.override.yml services: caddy: image: caddy:alpine command: caddy reverse-proxy --from <domain to serve on> --to backend:6666 ports: - 80:80 - 443:443 ``` If you `docker-compose up -d` with those two files, then you end up with a backend container and a reverse proxy for it with automatic HTTPS. We can supply a custom Caddyfile OR use Traefik as the reverse proxy if we need a more complicated setup
Quintasan (Migrated from github.com) reviewed 2024-05-03 01:59:18 +08:00
@@ -0,0 +1,35 @@
services:
Quintasan (Migrated from github.com) commented 2024-05-03 01:59:18 +08:00

I’ve never seen Docker compose set ups specifically bind the network interface. And, even on a VPS set up, you would need to specifically allow that port in the firewall; at least that’s how it has been for any fresh box I have installed.

@csmith1210 All OVH and Hetzner boxes I have had so far accept all connections on all ports, unless you explicitly configure them not to do so. Assuming your boxes don't (which is a good thing) accept all connections, then binding to 127.0.0.1:<port> changes exactly nothing in your case, while being a sane default for all boxes which don't have a firewall. I remain unconvinced that binding to 0.0.0.0 and exposing the backend container directly to the internet is a good idea.

I also think if most people are using a Docker set up, there’s no real need to “dumb it down”.

I don't think we're dumbing anything down here. We're trying to provide sane and secure defaults while giving people option to just run it without any tinkering. If someone wants to tinker with this setup, then they have all the escape hatches (docker-compose.override.yml and the ability to bring their own reverse proxy)

>I’ve never seen Docker compose set ups specifically bind the network interface. And, even on a VPS set up, you would need to specifically allow that port in the firewall; at least that’s how it has been for any fresh box I have installed. @csmith1210 All OVH and Hetzner boxes I have had so far accept all connections on all ports, unless you explicitly configure them not to do so. Assuming your boxes don't (which is a good thing) accept all connections, then binding to `127.0.0.1:<port>` changes exactly nothing in your case, while being a sane default for all boxes which don't have a firewall. I remain unconvinced that binding to `0.0.0.0` and exposing the backend container directly to the internet is a good idea. > I also think if most people are using a Docker set up, there’s no real need to “dumb it down”. I don't think we're dumbing anything down here. We're trying to provide sane and secure defaults while giving people option to `just run it` without any tinkering. If someone wants to tinker with this setup, then they have all the escape hatches (`docker-compose.override.yml` and the ability to bring their own reverse proxy)
gantoine (Migrated from github.com) reviewed 2024-05-03 22:29:17 +08:00
@@ -0,0 +1,35 @@
services:
gantoine (Migrated from github.com) commented 2024-05-03 22:29:17 +08:00

binding to 127.0.0.1: changes exactly nothing in your case, while being a sane default for all boxes which don't have a firewall.

This is a fair point, and as it personally wouldn't affect me (as I know how to edit a compose file) I think it's fine to set it that way in the default. 👍🏼

> binding to 127.0.0.1:<port> changes exactly nothing in your case, while being a sane default for all boxes which don't have a firewall. This is a fair point, and as it personally wouldn't affect me (as I know how to edit a compose file) I think it's fine to set it that way in the default. 👍🏼
adyanth (Migrated from github.com) reviewed 2024-05-04 22:25:29 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
adyanth (Migrated from github.com) commented 2024-05-04 22:25:29 +08:00

As long as the tags are semvar compatible, the existing tag scheme is very nice to be able to select an upgrade schedule using auto image pulls. One can have maybe:latest which always auto upgrades (and should not be done), while one can do maybe:2 or maybe:2.3 and get the patch upgrades.

As long as the tags are semvar compatible, the existing tag scheme is very nice to be able to select an upgrade schedule using auto image pulls. One can have `maybe:latest` which always auto upgrades (and should not be done), while one can do `maybe:2` or `maybe:2.3` and get the patch upgrades.
Radu-C-Martin (Migrated from github.com) reviewed 2024-05-05 01:33:31 +08:00
@@ -0,0 +1,107 @@
name: Publish Docker image
Radu-C-Martin (Migrated from github.com) commented 2024-05-05 01:33:31 +08:00

That's how I've seen it done in other projects. It's intuitive, and also works well with renovate/dependabot. But I guess that would impose stricter adherence to semver versioning to be totally useful, so it's more for the core maintainers to decide. It can always be added later :D

That's how I've seen it done in other projects. It's intuitive, and also works well with renovate/dependabot. But I guess that would impose stricter adherence to semver versioning to be totally useful, so it's more for the core maintainers to decide. It can always be added later :D
zachgoll (Migrated from github.com) reviewed 2024-05-06 21:50:49 +08:00
@@ -0,0 +1,35 @@
services:
zachgoll (Migrated from github.com) commented 2024-05-06 21:50:49 +08:00

I think @Quintasan's proposal here makes sense overall:

  1. Give a sane default that "just works"
  2. Provide escape hatches for those with the knowledge and/or motivation to customize

Still not 100% certain on the port binding (mostly because I've never seen or done this), but I'd rather be conservative to start and then make modifications (or allow the user to make those mods) if needed.

I think @Quintasan's proposal here makes sense overall: 1. Give a sane default that "just works" 2. Provide escape hatches for those with the knowledge and/or motivation to customize Still not 100% certain on the port binding (mostly because I've never seen or done this), but I'd rather be _conservative_ to start and then make modifications (or allow the user to make those mods) if needed.
zachgoll commented 2024-05-06 22:15:16 +08:00 (Migrated from github.com)

@Radu-C-Martin this looks good, thanks for the changes! The documentation and overall structure is good with me.

If I'm not mistaken, the only pending item we have left is the reverse proxy config and the "override" file mentioned by @Quintasan. I think this may be a good follow-up PR.

Unless there are objections, I think we can merge this as-is so we can start getting some images published to GHCR for self-hosters to start playing around with.

@Radu-C-Martin this looks good, thanks for the changes! The documentation and overall structure is good with me. If I'm not mistaken, the only pending item we have left is the reverse proxy config and the "override" file mentioned by @Quintasan. I think this may be a good follow-up PR. Unless there are objections, I think we can merge this as-is so we can start getting some images published to GHCR for self-hosters to start playing around with.
zachgoll (Migrated from github.com) approved these changes 2024-05-07 00:41:37 +08:00
Radu-C-Martin commented 2024-05-07 14:01:07 +08:00 (Migrated from github.com)

Hey, Zach! Thanks for merging my PR 🙂 I tried accessing the image and apparently I'm not authorized. I think there could be some settings missing in the organization settings? (this is what I found, but could be out of date: https://github.com/orgs/community/discussions/26014, I've never had to deal this yet). I see that the CI passes correctly, so I assume for you the image works at least?

Hey, Zach! Thanks for merging my PR 🙂 I tried accessing the image and apparently I'm not authorized. I think there could be some settings missing in the organization settings? (this is what I found, but could be out of date: https://github.com/orgs/community/discussions/26014, I've never had to deal this yet). I see that the CI passes correctly, so I assume for you the image works at least?
zachgoll commented 2024-05-07 19:48:26 +08:00 (Migrated from github.com)

@Radu-C-Martin thanks for the heads up, we'll get the org setting updated shortly.

@Radu-C-Martin thanks for the heads up, we'll get the org setting updated shortly.
Radu-C-Martin commented 2024-05-07 22:20:10 +08:00 (Migrated from github.com)

All good now!

All good now!
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gavin/maybe#640