k8s-at-home / charts

⚠️ Deprecated : Helm charts for applications you run at home
https://docs.k8s-at-home.com
Apache License 2.0
1.45k stars 623 forks source link

[dendrite] Per-component database settings #1581

Closed Omar007 closed 2 years ago

Omar007 commented 2 years ago

Description of the change

This change allows database configuration to be set for each of the sub-components of Dendrite.

Benefits

This enabled use-cases such as:

Possible drawbacks

None that I'm aware of. Backward compatibility is kept with the current logic.

Applicable issues

N/A

Additional information

N/A

Checklist

Omar007 commented 2 years ago

Looks like the CI is failing on something unrelated to this change? :thinking:

time="2022-05-23T06:09:42Z" level=info msg="Starting \"syncapi\" component"
time="2022-05-23T06:09:42Z" level=error msg="Configuration error: You have tried to enable open registration without any secondary verification methods (such as reCAPTCHA). By enabling open registration, you are SIGNIFICANTLY increasing the risk that your server will be used to send spam or abuse, and may result in your server being banned from some rooms. If you are ABSOLUTELY CERTAIN you want to do this, start Dendrite with the -really-enable-open-registration command line flag. Otherwise, you should set the registration_disabled option in your Dendrite config."
time="2022-05-23T06:09:42Z" level=fatal msg="Failed to start due to configuration errors"
Jonnobrow commented 2 years ago

For some reason the syncapi is pulling the latest version rather than v0.8.1. In v0.8.3 there was a modification to make open registration harder.

So to fix:

EDIT: Seem like an easy fix, I must have added it by mistake here: https://github.com/k8s-at-home/charts/pull/1451/files

Jonnobrow commented 2 years ago

I have opened a PR with a fix: https://github.com/Omar007/charts/pull/1

Omar007 commented 2 years ago

I have cherry-picked the one commit with the changes for dendrite out of that PR and added it here.

truxnell commented 2 years ago

LGTM. However, are we confident it wont be breaking to users - i.e. is a minor bump appropriate? It does look to me like it wont be breaking however i do note this PR would change 'registration_disabled' from false to true for users?

Omar007 commented 2 years ago

Yea I suppose with those fixes maybe the version should be just bumped to 5.0.0 :thinking:

truxnell commented 2 years ago

I may be taking a over-abundance of caution, but do we do similar to the review, a major semver + callout the registration change? Happy to go either way, I'm not familiar with the chart, but we do like to follow SemVer closely.

Omar007 commented 2 years ago

I have bumped it up to 0.8.7 due do some important fixes in Dendrite itself and included both chart suggestions bumping it to 5.0.0 and explicitly mentioning the defaults change in the changelog :+1:

truxnell commented 2 years ago

Thanks for the PR @Omar007 & assistance @Jonnobrow!

truxnell commented 2 years ago

@all-contributors, can you add @Omar007 for code.

truxnell commented 2 years ago

I summon thee from the firey depth of hell @all-contributors!
By thine divine glory add @Omar007 for code contributions to this, our most holy of repositories.

allcontributors[bot] commented 2 years ago

@Truxnell

I've put up a pull request to add @Omar007! :tada:

truxnell commented 2 years ago

And now begone, to thence which you have came @all-contributors! I banish thee, once, twice, thrice! Abandon this plane of existence and leave us mortals in peace. I bind yee to the nine trees of eternity once more until my wishes desire it.