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.

Sign up for GitHub

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

Jump to bottom

Raise errors if both externalService and service are set #319

Merged
merged 4 commits into from
Mar 8, 2022
Merged

Raise errors if both externalService and service are set #319

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c955a62
Raise errors if both externalService and service are set
aaronandrino Mar 4, 2022
ad2bb27
Merged tpl files for invalid values
aaronandrino Mar 7, 2022
e245d24
Add test coverage for error-on-invalid-values.yaml
aaronandrino Mar 7, 2022
73923d8
Merged error-on-invalid-values snippets
aaronandrino Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions charts/posthog/templates/_snippet-error-on-both-externalService-and-service-enabled.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{{/* Raise an error if externalService is configured without service.enabled being false. */}}
{{- define "snippet.error-on-both-externalService-and-service-enabled" }}
{{- if and .Values.postgresql.enabled .Values.externalPostgresql.postgresqlHost }}
{{- required (printf (include "snippet.error-on-both-externalService-and-service-enabled-template" .) "externalPostgresql.postgresqlHost cannot be set if postgresql.enabled is true") nil -}}
{{- else if and .Values.redis.enabled .Values.externalRedis.host }}
{{- required (printf (include "snippet.error-on-both-externalService-and-service-enabled-template" .) "externalRedis.host cannot be set if redis.enabled is true") nil -}}
{{- else if and .Values.kafka.enabled .Values.externalKafka.brokers }}
{{- required (printf (include "snippet.error-on-both-externalService-and-service-enabled-template" .) "externalKafka.brokers cannot be set if kafka.enabled is true") nil -}}
{{- else if and .Values.clickhouse.enabled .Values.externalClickhouse.host }}
{{- required (printf (include "snippet.error-on-both-externalService-and-service-enabled-template" .) "externalClickhouse.host cannot be set if clickhouse.enabled is true") nil -}}
{{- else if and .Values.clickhouse.enabled .Values.externalClickhouse.cluster }}
{{- required (printf (include "snippet.error-on-both-externalService-and-service-enabled-template" .) "externalClickhouse.cluster cannot be set if clickhouse.enabled is true") nil -}}
{{- end -}}
{{- end -}}

{{- define "snippet.error-on-both-externalService-and-service-enabled-template" }}


==== MISCONFIGURATION ERROR ====

Read more on how to update your values.yaml:

- %s

=========================

{{ end -}}
5 changes: 5 additions & 0 deletions charts/posthog/templates/error-on-both-externalService-and-service-enabled.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{{- if false -}}
# :TRICKY: This template raises an error if any externalService is configured without service.enabled being false in values.yaml.
# It doesn't actually define any real kubernetes resources
{{- end -}}
{{- include "snippet.error-on-both-externalService-and-service-enabled" . -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor request: please merge this file with charts/posthog/templates/error-on-removed-values.yaml to charts/posthog/templates/error-on-invalid-values.yaml, merge the tpl file with the other tpl file.

After that this looks great to me. It would be awesome to add some helm-unittest coverage here, but not required immediately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I couldn't add an error test case because of this bug, but it should be possible to do so once the bug fix is released.