feat: v4.38.0 #215
feat: v4.38.0 #215
Conversation
james-d-elliott
force-pushed
the
feat-4.38.0
branch
5 times, most recently
from
0a1e5ab
to
713ebac
Compare
james-d-elliott
force-pushed
the
feat-4.38.0
branch
3 times, most recently
from
bb2f713
to
6988c76
Compare
james-d-elliott
force-pushed
the
feat-4.38.0
branch
from
6988c76
to
b752380
Compare
james-d-elliott
force-pushed
the
feat-4.38.0
branch
from
2eb9f41
to
9f4cb93
Compare
james-d-elliott
force-pushed
the
feat-4.38.0
branch
from
9f4cb93
to
0383037
Compare
WalkthroughThe version 0.9.0-beta1 update for the Authelia Helm chart introduces significant enhancements and breaking changes. This update adds dependencies on PostgreSQL, MariaDB, and Redis, overhauls secret management, restructures sessions and domains, updates OIDC 1.0 configurations, and revises various templates and configurations for better clarity and functionality. Notable changes also include updates to ingress configurations and the introduction of validation checks for configurations and secrets. Changes
This table provides a concise overview of the key changes across different files in the Authelia Helm chart update, highlighting the breadth and depth of the enhancements made in this version. Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (34)
Files not summarized due to errors (2)
Files skipped from review due to trivial changes (2)
Additional Context UsedLanguageTool (876)
Additional comments not posted (91)
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 (
|
james-d-elliott
force-pushed
the
feat-4.38.0
branch
from
1c583c2
to
34002bf
Compare
james-d-elliott
force-pushed
the
feat-4.38.0
branch
from
34002bf
to
2310432
Compare
* Fixes to allow authelia to start * Changes to make lint succeed. * Correct environment variable na.e * Rollback changes to values.yaml, change validations to not validate on .enabled just existance of the parent key. change some templates to cope with missing parent key. * My values now load - validations of oidc client_secret fixed.
* Add support for integrated MariaDB and PostgreSQL * Bump chart version * Update values.yaml to retain behaviour Signed-off-by: Jonathan <me@jonathangazeley.com> --------- Signed-off-by: Jonathan <me@jonathangazeley.com>
* Add support for integrated Redis * Update charts/authelia/Chart.yaml Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com> --------- Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com> Co-authored-by: James Elliott <james-d-elliott@users.noreply.github.com>
|
@james-d-elliott Do you have a timeline for releasing v4.38.0? Thanks 🙂 |
* Fix wrapYAML indentation
This was generating an invalid access control yaml file:
---
access_control:
default_policy: 'deny'
rules:
- policy: one_factor
domain:
- '*.example.com'
...
* Fix oidc jwk indentation
This would generate a secret template with an invalid identation:
jwks:
key: {{ secret "/secrets/authelia-jwks/private.pem" | mindent 8 "|" | msquote }}
Causing the following error:
Configuration: failed to load configuration from file
path(/configuration.yaml) source: yaml: line 158: could not find
expected ':'
* Update charts/authelia/templates/configMap.yaml
Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>
---------
Signed-off-by: James Elliott <james-d-elliott@users.noreply.github.com>
Co-authored-by: James Elliott <james-d-elliott@users.noreply.github.com>
|
We're looking for feedback on the chart, including the particular elements they are using, the accuracy of the breaking changes listed here, and the consistency of the new format for secrets. |
Allow overriding the kube DNS Domain.
|
Failed to pull image "ghcr.io/authelia/authelia:v4.38.6" Per default it also seems to deploy mariadb, postgresql & redis in the cluster, even when those things are disabled in the storage section. Shouldn't mariadb & postgresql be mutually exclusive anyway? |
|
Fixed, testing value made it into the chart whoops! However I'd prefer to keep feedback in this thread if possible: #220 |
|
Merging this as a beta. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -36,91 +36,78 @@ image: | |||
| # registry: docker.io | |||
| registry: ghcr.io | |||
| repository: authelia/authelia | |||
| tag: "" | |||
| tag: '' | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The verification script has successfully identified that the tag field in the charts/authelia/values.local.yaml file is indeed set to an empty string (tag: ''). This confirms the concern raised in the review comment about the potential issue with pulling the default latest tag, which might not be stable.
Conclusion:
The Docker image tag is not specified in the tag field of the charts/authelia/values.local.yaml file. It is crucial to specify a stable and specific version of the Docker image to avoid potential issues with the default latest tag.
- File:
charts/authelia/values.local.yaml - Issue: The
tagfield is empty.
Analysis chain
Ensure the Docker image tag is specified in the tag field to avoid issues with pulling the default latest tag, which might not be stable.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the Docker image tag is specified and not left empty.
rg --type yaml 'tag: \'\'' charts/authelia/values.local.yaml
Length of output: 93
Script:
#!/bin/bash
# Verify that the Docker image tag is specified and not left empty.
rg --type yaml "tag: ''" charts/authelia/values.local.yaml
Length of output: 76
| @@ -18,15 +14,18 @@ | |||
| ## | |||
| ## Important Points: | |||
| ## - No guarantees of support for prior versions is given. The chart is intended to be used with the AppVersion. | |||
| ## - Does not and will not support any version prior to 4.30.0 due to a significant refactor of the configuration | |||
| ## - Does not and will not support any version prior to 4.38.0 due to a significant refactor of the configuration | |||
There was a problem hiding this comment.
Ensure backward compatibility or provide a clear migration path for versions prior to 4.38.0.
| @@ -35,91 +34,85 @@ image: | |||
| # registry: docker.io | |||
| registry: ghcr.io | |||
| repository: authelia/authelia | |||
| tag: "" | |||
| tag: '' | |||
There was a problem hiding this comment.
Specify a default Docker image tag to avoid accidental deployments of incorrect or unstable versions.
| {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}} | ||
| {{- .Values.ingress.traefikCRD.middlewares.auth.nameOverride | trunc 63 -}} | ||
| {{- else -}} | ||
| {{- (printf "forwardauth-%s" (include "authelia.name" .)) | trunc 63 -}} | ||
| {{- end -}} | ||
| {{- else -}} | ||
| {{- $name := .Name | trimPrefix "forward-auth-" -}} | ||
| {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}} | ||
| {{- (printf "%s-%s" .Values.ingress.traefikCRD.middlewares.auth.nameOverride $name) | trunc 63 -}} | ||
| {{- else -}} | ||
| {{- (printf "forwardauth-%s-%s" (include "authelia.name" .) $name) | trunc 63 -}} | ||
| {{- end -}} | ||
| {{- end -}} |
There was a problem hiding this comment.
Refactor middleware name generation to reduce redundancy.
The logic for generating middleware names is duplicated with slight variations. Consider extracting the common logic into a helper function to improve maintainability and reduce code duplication.
- {{- if eq .Name "forward-auth" -}}
- {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}}
- {{- .Values.ingress.traefikCRD.middlewares.auth.nameOverride | trunc 63 -}}
- {{- else -}}
- {{- (printf "forwardauth-%s" (include "authelia.name" .)) | trunc 63 -}}
- {{- end -}}
- {{- else -}}
- {{- $name := .Name | trimPrefix "forward-auth-" -}}
- {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}}
- {{- (printf "%s-%s" .Values.ingress.traefikCRD.middlewares.auth.nameOverride $name) | trunc 63 -}}
- {{- else -}}
- {{- (printf "forwardauth-%s-%s" (include "authelia.name" .) $name) | trunc 63 -}}
- {{- end -}}
- {{- end -}}
+ {{- $baseName := default "forwardauth" .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}}
+ {{- $suffix := default "" .Name | trimPrefix "forward-auth-" -}}
+ {{- printf "%s-%s" $baseName $suffix | trunc 63 -}}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}} | |
| {{- .Values.ingress.traefikCRD.middlewares.auth.nameOverride | trunc 63 -}} | |
| {{- else -}} | |
| {{- (printf "forwardauth-%s" (include "authelia.name" .)) | trunc 63 -}} | |
| {{- end -}} | |
| {{- else -}} | |
| {{- $name := .Name | trimPrefix "forward-auth-" -}} | |
| {{- if .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}} | |
| {{- (printf "%s-%s" .Values.ingress.traefikCRD.middlewares.auth.nameOverride $name) | trunc 63 -}} | |
| {{- else -}} | |
| {{- (printf "forwardauth-%s-%s" (include "authelia.name" .) $name) | trunc 63 -}} | |
| {{- end -}} | |
| {{- end -}} | |
| {{- $baseName := default "forwardauth" .Values.ingress.traefikCRD.middlewares.auth.nameOverride -}} | |
| {{- $suffix := default "" .Name | trimPrefix "forward-auth-" -}} | |
| {{- printf "%s-%s" $baseName $suffix | trunc 63 -}} |
| While we have aimed to keep documented backwards compatability for previous versions of Authelia deployed with the chart | ||
| we have to draw a line with this release. Due to the way the chart was designed and the introduction of mutli-cookie | ||
| domains and the new authz endpoints there is just too many scenarios to handle. We're therefore cutting off support for |
There was a problem hiding this comment.
Correct spelling mistakes and grammatical errors.
There are several spelling and grammatical errors in the text that need correction for clarity and professionalism.
- compatability
+ compatibility
- mutli-cookie
+ multi-cookieCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| While we have aimed to keep documented backwards compatability for previous versions of Authelia deployed with the chart | |
| we have to draw a line with this release. Due to the way the chart was designed and the introduction of mutli-cookie | |
| domains and the new authz endpoints there is just too many scenarios to handle. We're therefore cutting off support for | |
| While we have aimed to keep documented backwards compatibility for previous versions of Authelia deployed with the chart | |
| we have to draw a line with this release. Due to the way the chart was designed and the introduction of multi-cookie | |
| domains and the new authz endpoints there is just too many scenarios to handle. We're therefore cutting off support for |
| {{- define "authelia.func.dquote" }} | ||
| {{- if kindIs "string" . }} | ||
| {{- . | squote }} | ||
| {{- else }} | ||
| {{- . }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Tip
Codebase Verification
The authelia.func.dquote function in the Helm chart template is designed to quote strings using single quotes, despite its name suggesting double quotes (dquote). This could lead to confusion or errors if the function's name is taken to imply the type of quoting it performs. Here are the details:
- The function checks if the input is a string and applies single quoting (
squote). - For non-string inputs, it returns the input unchanged.
This behavior might not align with the expectations set by the function's name, suggesting a review and possible renaming of the function to accurately reflect its operation might be necessary.
Analysis chain
Check the implementation of authelia.func.dquote to ensure it correctly handles quoting of strings, especially in edge cases.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the behavior of the dquote function with various input types.
cat charts/authelia/templates/_misc.tpl | grep "authelia.func.dquote"
Length of output: 106
Script:
#!/bin/bash
# Extract the full content of the 'authelia.func.dquote' function from the '_misc.tpl' file
sed -n '/{{- define "authelia.func.dquote" }}/,/{{- end }}/p' charts/authelia/templates/_misc.tpl
Length of output: 242
| {{- if not .Values.configMap.disabled -}} | ||
| {{- if .Values.configMap.authentication_backend.file.enabled -}} | ||
| {{- true -}} | ||
| {{- else if .Values.configMap.storage.local.enabled -}} | ||
| {{- else if and (.Values.configMap.storage.local) (.Values.configMap.storage.local.enabled) -}} | ||
| {{- true -}} | ||
| {{- else if not .Values.configMap.session.redis.enabled -}} | ||
| {{- else if not (and (.Values.configMap.session.redis) (.Values.configMap.session.redis.enabled)) -}} | ||
| {{- true -}} | ||
| {{- else if and (not .Values.configMap.storage.mysql.enabled) (not .Values.configMap.storage.postgres.enabled) -}} | ||
| {{- else if and (not (and (.Values.configMap.storage.mysql) (.Values.configMap.storage.mysql.enabled))) (not (and (.Values.configMap.storage.postgres) (.Values.configMap.storage.postgres.enabled))) -}} | ||
| {{- true -}} | ||
| {{- else if not .Values.configMap.authentication_backend.ldap.enabled -}} | ||
| {{- else if not (and (.Values.configMap.authentication_backend) (.Values.configMap.authentication_backend.ldap.enabled)) -}} |
There was a problem hiding this comment.
Simplify the logic for determining if a pod is stateful.
The current implementation for determining if a pod is stateful involves multiple nested conditions. Consider simplifying this logic by consolidating conditions or using helper functions to improve readability and maintainability.
- {{- if not .Values.configMap.disabled -}}
- {{- if .Values.configMap.authentication_backend.file.enabled -}}
- {{- true -}}
- {{- else if and (.Values.configMap.storage.local) (.Values.configMap.storage.local.enabled) -}}
- {{- true -}}
- {{- else if not (and (.Values.configMap.session.redis) (.Values.configMap.session.redis.enabled)) -}}
- {{- true -}}
- {{- else if and (not (and (.Values.configMap.storage.mysql) (.Values.configMap.storage.mysql.enabled))) (not (and (.Values.configMap.storage.postgres) (.Values.configMap.storage.postgres.enabled))) -}}
- {{- true -}}
- {{- else if not (and (.Values.configMap.authentication_backend) (.Values.configMap.authentication_backend.ldap.enabled)) -}}
- {{- true -}}
- {{- end -}}
- {{- end -}}
+ {{- $isStateful := or
+ (.Values.configMap.authentication_backend.file.enabled)
+ (and (.Values.configMap.storage.local) (.Values.configMap.storage.local.enabled))
+ (not (and (.Values.configMap.session.redis) (.Values.configMap.session.redis.enabled)))
+ (and (not (and (.Values.configMap.storage.mysql) (.Values.configMap.storage.mysql.enabled))) (not (and (.Values.configMap.storage.postgres) (.Values.configMap.storage.postgres.enabled))))
+ (not (and (.Values.configMap.authentication_backend) (.Values.configMap.authentication_backend.ldap.enabled)))
+ -}}
+ {{- if not .Values.configMap.disabled -}}
+ {{- $isStateful -}}
+ {{- end -}}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| {{- if not .Values.configMap.disabled -}} | |
| {{- if .Values.configMap.authentication_backend.file.enabled -}} | |
| {{- true -}} | |
| {{- else if .Values.configMap.storage.local.enabled -}} | |
| {{- else if and (.Values.configMap.storage.local) (.Values.configMap.storage.local.enabled) -}} | |
| {{- true -}} | |
| {{- else if not .Values.configMap.session.redis.enabled -}} | |
| {{- else if not (and (.Values.configMap.session.redis) (.Values.configMap.session.redis.enabled)) -}} | |
| {{- true -}} | |
| {{- else if and (not .Values.configMap.storage.mysql.enabled) (not .Values.configMap.storage.postgres.enabled) -}} | |
| {{- else if and (not (and (.Values.configMap.storage.mysql) (.Values.configMap.storage.mysql.enabled))) (not (and (.Values.configMap.storage.postgres) (.Values.configMap.storage.postgres.enabled))) -}} | |
| {{- true -}} | |
| {{- else if not .Values.configMap.authentication_backend.ldap.enabled -}} | |
| {{- else if not (and (.Values.configMap.authentication_backend) (.Values.configMap.authentication_backend.ldap.enabled)) -}} | |
| {{- $isStateful := or | |
| (.Values.configMap.authentication_backend.file.enabled) | |
| (and (.Values.configMap.storage.local) (.Values.configMap.storage.local.enabled)) | |
| (not (and (.Values.configMap.session.redis) (.Values.configMap.session.redis.enabled))) | |
| (and (not (and (.Values.configMap.storage.mysql) (.Values.configMap.storage.mysql.enabled))) (not (and (.Values.configMap.storage.postgres) (.Values.configMap.storage.postgres.enabled)))) | |
| (not (and (.Values.configMap.authentication_backend) (.Values.configMap.authentication_backend.ldap.enabled))) | |
| -}} | |
| {{- if not .Values.configMap.disabled -}} | |
| {{- $isStateful -}} | |
| {{- end -}} |
This is a work in progress chart version for 4.38.0 (chart version will be 0.9.0 and a BREAKING CHANGE). Public discussion is welcome in this thread.
Summary by CodeRabbit