-
Notifications
You must be signed in to change notification settings - Fork 49
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
Bundle Helm charts for Redis, MariaDB and PostgreSQL #244
Conversation
Warning Rate limit exceeded@djjudas21 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 55 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe primary change introduced is the enhancement and expansion of the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant HelmChart as Authelia Helm Chart
participant Redis as Redis Subchart
participant PostgreSQL as PostgreSQL Subchart
participant MariaDB as MariaDB Subchart
User->>HelmChart: Deploy Authelia with optional dependencies
alt Redis Enabled
HelmChart->>Redis: Deploy Redis
end
alt PostgreSQL Enabled
HelmChart->>PostgreSQL: Deploy PostgreSQL
end
alt MariaDB Enabled
HelmChart->>MariaDB: Deploy MariaDB
end
Note right of MariaDB: Optional subcharts deployed based on user configuration
Assessment against linked issues
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Outside diff range and nitpick comments (19)
charts/authelia/README.md (19)
Line range hint
13-13
: Consider putting a comma before the abbreviation “i.e.”.To improve readability, add a comma before "i.e.".
- breaking changes within minor releases i.e. from v0.1.0 to v0.2.0. + breaking changes within minor releases, i.e., from v0.1.0 to v0.2.0.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
24-24
: Possible typo: repeated word.There is a repetition of the word "values".
- Install the chart with `helm install authelia authelia/authelia` and optionally set your values with `--values values.yaml` or via `--set [parameter]=[value]`. + Install the chart with `helm install authelia authelia/authelia` and optionally set your values with `--values values.yaml` or via `--set [parameter]=[value]`.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
49-49
: Consider putting a comma before the abbreviation “i.e.”.To improve readability, add a comma before "i.e.".
- value i.e. you can have one blank and one configured but all those that are configured must be the same, and in addition if configured at all the `configMap.server.path` option must have the same value. + value, i.e., you can have one blank and one configured, but all those that are configured must be the same, and in addition, if configured at all, the `configMap.server.path` option must have the same value.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
66-66
: Consider replacing "quite large" with a more concise phrase.To improve readability, consider replacing "quite large" with a more concise phrase.
- As the chart values are quite large, we've split it into sections. + As the chart values are extensive, we've split it into sections.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
108-110
: Use hyphen in "based-affinity" and "based-pods".To improve readability, use hyphens in "based-affinity" and "based-pods".
- Configures the pod to select nodes based affinity + Configures the pod to select nodes based-affinity - Configures the pod to select nodes based pods on the node + Configures the pod to select nodes based-pods on the nodeTools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
148-148
: Separate "otherwise" from the sentence.To improve readability, separate "otherwise" from the sentence.
- If true generates the ConfigMap, otherwise it doesn't + If true, generates the ConfigMap; otherwise, it doesn'tTools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
178-178
: Use a comma before "but".To improve readability, use a comma before "but".
- There are likely ways around this but we do not recommend it. + There are likely ways around this, but we do not recommend it.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
179-179
: Separate "however" from the sentence.To improve readability, separate "however" from the sentence.
- if the value is not defined, however we recommend manually generating the secret + if the value is not defined; however, we recommend manually generating the secretTools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
182-182
: Consider simply using “of” instead.To improve readability, consider simply using "of" instead.
- can be one of any of the following values + can be one of the following valuesTools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
201-201
: Consider removing “of” to be more concise.To improve readability, consider removing "of" to be more concise.
- The path where to mount all of the secrets + The path where to mount all the secretsTools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
30-30
: Remove trailing space.Remove the trailing space to comply with Markdownlint rules.
-
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
122-122
: Remove trailing space.Remove the trailing space to comply with Markdownlint rules.
-
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
19-19
: Remove multiple top-level headings.Remove the multiple top-level headings to comply with Markdownlint rules.
-
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
64-64
: Remove multiple top-level headings.Remove the multiple top-level headings to comply with Markdownlint rules.
-
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
206-206
: Remove multiple top-level headings.Remove the multiple top-level headings to comply with Markdownlint rules.
-
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
21-21
: Avoid using bare URLs.Use proper Markdown link syntax instead of bare URLs to comply with Markdownlint rules.
- Visit https://charts.authelia.com and follow the instructions to install the chart repo. + Visit [Authelia Charts](https://charts.authelia.com) and follow the instructions to install the chart repo.Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
237-237
: Avoid using bare URLs.Use proper Markdown link syntax instead of bare URLs to comply with Markdownlint rules.
- [chart-testing](https://github.com/helm/chart-testing)
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
238-238
: Avoid using bare URLs.Use proper Markdown link syntax instead of bare URLs to comply with Markdownlint rules.
- [chart-releaser](https://github.com/helm/chart-releaser)
Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Line range hint
143-143
: Use underscores for emphasis.Use underscores for emphasis to comply with Markdownlint rules.
- *Authelia* + _Authelia_Tools
Markdownlint
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
charts/authelia/Chart.yaml
is excluded by!**/*.yaml
charts/authelia/values.yaml
is excluded by!**/*.yaml
Files selected for processing (2)
- charts/authelia/.helmignore (1 hunks)
- charts/authelia/README.md (2 hunks)
Files skipped from review due to trivial changes (1)
- charts/authelia/.helmignore
Additional context used
LanguageTool
charts/authelia/README.md
[typographical] ~13-~13: Consider putting a comma before the abbreviation “i.e.”.
Context: ... keep the breaking changes within minor releases i.e. from v0.1.0 to v0.2.0. The following ve...(IE_COMMA)
[duplication] ~24-~24: Possible typo: you repeated a word
Context: ...he next step. 3. Install the chart withhelm install authelia authelia/authelia
and optionally set your value...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~24-~24: Possible typo: you repeated a word
Context: ...iaand optionally set your values with
--values values.yamlor via
--set [parameter]=[val...(ENGLISH_WORD_REPEAT_RULE)
[style] ~39-~39: ‘a majority of the’ might be wordy. Consider a shorter alternative.
Context: ...onfigMap section (the configMap follows a majority of the configuration options in [the documen...(EN_WORDINESS_PREMIUM_A_MAJORITY_OF_THE)
[typographical] ~49-~49: Consider putting a comma before the abbreviation “i.e.”.
Context: ...ion configured MUST have the same value i.e. you can have one blank and one configur...(IE_COMMA)
[style] ~66-~66: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...e chart values. As the chart values are quite large, we've split it into sections. ## Gene...(EN_WEAK_ADJECTIVE)
[uncategorized] ~108-~108: This expression is usually spelled with a hyphen.
Context: ... | Configures the pod to select nodes based affinity | {} | | ...(BASED_HYPHEN)
[uncategorized] ~109-~109: This expression is usually spelled with a hyphen.
Context: ...ty | Configures the pod to select nodes based pods on the node | {} | |...(BASED_HYPHEN)
[uncategorized] ~110-~110: This expression is usually spelled with a hyphen.
Context: ...nity | Configures the pod to select nodes based pods on the node | {} | |...(BASED_HYPHEN)
[typographical] ~148-~148: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ... | If true generates the ConfigMap, otherwise it doesn't | true | | ...(THUS_SENTENCE)
[uncategorized] ~178-~178: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...chart. There are likely ways around this but we do not recommend it. Most secrets ar...(COMMA_COMPOUND_SENTENCE)
[typographical] ~179-~179: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ly generated if the value is not defined, however we recommend manually generating the se...(HOWEVER_SENTENCE)
[style] ~182-~182: Consider simply using “of” instead.
Context: ...isn't leaked. The*
below can be one of any of the following values: - jwt - ldap - s...(OF_ANY_OF)
[style] ~201-~201: Consider removing “of” to be more concise
Context: ...Path | The path where to mount all of the secrets | /secrets | | secret...(ALL_OF_THE)
Markdownlint
charts/authelia/README.md
30-30: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
122-122: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
171-171: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
10-10: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
19-19: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
64-64: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
206-206: null
Multiple top-level headings in the same document(MD025, single-title, single-h1)
21-21: null
Bare URL used(MD034, no-bare-urls)
237-237: null
Bare URL used(MD034, no-bare-urls)
238-238: null
Bare URL used(MD034, no-bare-urls)
143-143: Expected: underscore; Actual: asterisk
Emphasis style(MD049, emphasis-style)
143-143: Expected: underscore; Actual: asterisk
Emphasis style(MD049, emphasis-style)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Jonathan <me@jonathangazeley.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Jonathan <me@jonathangazeley.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Jonathan <me@jonathangazeley.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR adds basic support for deploying Redis, MariaDB or PostgreSQL as part of an Authelia deployment.
Typically the dependency subcharts would be integrated under the top level
mariadb:
orpostgresql:
keys, where the config can be inherited by the subcharts, and the main app (i.e. Authelia) would also draw its config from those keys. However the database config in the chart is understorage.mysql
andstorage.postgres
and changing this would be a breaking change.So I have added booleans
storage.redis.deploy
,storage.mysql.deploy
andstorage.postgres.deploy
which enable the user to deploy the Bitnami subcharts. This does not affect any existing Authelia deployments. At the moment the user has to copy a few settings (e.g. credentials) from themariadb
section to thestorage.mysql
section.In future we could consider a tighter integration, but it would be a more major refactoring, so let's go with this for now.
I have tested this chart on my own deployment with MariaDB and PostgreSQL databases, plus Redis.