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

WIP feature(demo): add a example for cms #41

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

DexterYan
Copy link
Member

@DexterYan DexterYan commented Mar 24, 2025

@DexterYan DexterYan marked this pull request as draft March 24, 2025 05:06
Copy link
Member

@chris-sanders chris-sanders left a comment

Choose a reason for hiding this comment

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

Why is the application name "cms" when the charts and files all seem to call it strapi? I think it should just be moved to match the application name and not a 'category' of software which is what I believe it is currently.

I know this is still marked draft, but I suggest you work on the Readme sooner than later. Thinking through a clearly communicated readme and any docs can help others give feedback and be used to set a milestone for what is in-scope for this portion of the PR.

Comment on lines +33 to +42
secret:
enabled: true
data:
APP_KEYS: "toBeModified1,toBeModified2"
API_TOKEN_SALT: tobemodified
ADMIN_JWT_SECRET: tobemodified
TRANSFER_TOKEN_SALT: tobemodified
JWT_SECRET: tobemodified
DATABASE_USERNAME: strapi
DATABASE_PASSWORD: strapi
Copy link
Member

Choose a reason for hiding this comment

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

Secrets shouldn't be in plain text, does this support the use of an external existing secret?

Comment on lines +128 to +130
auth:
username: strapi
password: strapi
Copy link
Member

Choose a reason for hiding this comment

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

As above can this reference an existing secret?
Another alternative is to auto-generate a secret and use that in the chart so that there isn't a global default people accidentally use.

affinity: {}

# -- Embedded Postgres configuration
# Deploys a cluster using the CloudnativePG Operator
Copy link
Member

Choose a reason for hiding this comment

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

Does this / will this / include an option to use an existing external db instead?

Comment on lines +3 to +4
includes:
utils: ./taskfiles/utils.yml
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there are any good options for us to keep taskfiles in sync if we land on a similar pattern. Maybe the includes provides us a way to do that?

I wasn't 100% sold on the 'includes' pattern I hit a few issues with having to pass variables around, did you hit any complications with this or did it work fine for you?

How did you get this bootstrapped did you basically copy/paste/modify from wg_easy? That might be sufficient at least to start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants