fix for OAUTH_ENABLED check #17

Merged
jmakdah2 merged 1 commits from patch_5.1.1_release into main 2025-08-03 22:55:55 +00:00
3 changed files with 10 additions and 4 deletions

View File

@ -1,4 +1,4 @@
export LOOMIO_ENTRYPOINT_VERSION=v7
export LOOMIO_ENTRYPOINT_VERSION=v8
# cannot be integrated into entrypoint.sh as it requires the operator to create a user first
function make_last_user_admin()

View File

@ -11,6 +11,7 @@ x-oauth-env: &oauth-env
OAUTH_ATTR_NAME:
OAUTH_ATTR_EMAIL:
OAUTH_LOGIN_PROVIDER_NAME:
OAUTH_ENABLED:
services:
app:
@ -18,6 +19,11 @@ services:
*oauth-env
secrets:
- oauth_app_secret
worker:
environment:
*oauth-env
secrets:
- oauth_app_secret
secrets:
oauth_app_secret:

View File

@ -26,9 +26,9 @@ file_env "SECRET_COOKIE_TOKEN"
file_env "POSTGRES_PASSWORD"
file_env "SMTP_PASSWORD"
{{ if eq (env "OAUTH_ENABLED") "1" }}
file_env "OAUTH_APP_SECRET"
{{ end }}
if [ "$OAUTH_ENABLED" = "1" ]; then
decentral1se marked this conversation as resolved
Review

Is there a reason you're not templating it? I would assume it would be better to stick to convention?

E.g.

compose.yml Lines 133 to 136 in 2744684292
app_entrypoint:
name: ${STACK_NAME}_app_entrypoint_${APP_ENTRYPOINT_VERSION}
file: entrypoint.sh.tmpl
template_driver: golang

The .tmpl in https://git.coopcloud.tech/coop-cloud/peertube/src/branch/main/entrypoint.sh.tmpl

Another one of our weird recipe-isms 😂

Is there a reason you're not templating it? I would assume it would be better to stick to convention? E.g. https://git.coopcloud.tech/coop-cloud/peertube/src/commit/2744684292d66053a9681ac57692b9f026863dde/compose.yml#L133-L136 The `.tmpl` in https://git.coopcloud.tech/coop-cloud/peertube/src/branch/main/entrypoint.sh.tmpl Another one of our weird recipe-isms 😂
Review

Is this as simple as changing the file name to be end with .tmpl btw? @decentral1se

Is this as simple as changing the file name to be end with .tmpl btw? @decentral1se
Review

No reason, I was just working off of whatever was already there in the loomio recipe and didn't realize the convention is to have a .tmpl file for the entrypoint.

What are the benefits of it being a .tmpl file? Would you be ok with updating to .tmpl format in a separate change?

No reason, I was just working off of whatever was already there in the loomio recipe and didn't realize the convention is to have a .tmpl file for the entrypoint. What are the benefits of it being a .tmpl file? Would you be ok with updating to .tmpl format in a separate change?
Review

@ammaratef45 @jmakdah2 The advantage of the .tmpl file is that you can inject values into it via the template engine (e.g. the {{ if eq (env "OAUTH_ENABLED") "1" }} syntax works). The .tmpl file naming is just convention also, the main thing required is the compose.yml snippet i shared in #17 (comment) where you specify the template_driver. It's ultimately up to y'all. I also have no strong feelings on how changes are done in this PR or whatever, up to you!

@ammaratef45 @jmakdah2 The advantage of the `.tmpl` file is that you can inject values into it via the template engine (e.g. the `{{ if eq (env "OAUTH_ENABLED") "1" }}` syntax works). The `.tmpl` file naming is just convention also, the main thing required is the `compose.yml` snippet i shared in https://git.coopcloud.tech/coop-cloud/loomio/pulls/17#issuecomment-25428 where you specify the `template_driver`. It's ultimately up to y'all. I also have no strong feelings on how changes are done in this PR or whatever, up to you!
Review

thanks @decentral1se, I agree would be better to stick to convention, but it doesn't feel like this is impacting us much now so I can look into updating it with another change or separately after this PR!

thanks @decentral1se, I agree would be better to stick to convention, but it doesn't feel like this is impacting us much now so I can look into updating it with another change or separately after this PR!
file_env "OAUTH_APP_SECRET"
fi
export DB_HOST="db"
export DATABASE_URL="postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@db/${POSTGRES_DB}"