Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 1 addition & 1 deletion charts/external-dns/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ spec:
{{- if .Values.txtPrefix }}
- --txt-prefix={{ .Values.txtPrefix }}
{{- end }}
{{- if and (eq .Values.txtPrefix "") (ne .Values.txtSuffix "") }}
{{- if and (not (empty .Values.txtSuffix)) (empty .Values.txtPrefix) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for the sake of simplicity this should be linked together.

{{- if and .Values.txtPrefix .Values.txtSuffix }}
{{- fail (printf "'txtPrefix' and 'txtSuffix' are mutually exclusive") }}
{{- end }}
{{- if .Values.txtPrefix }}
- --txt-prefix={{ .Values.txtPrefix }}
{{- else if .Values.txtSuffix }}
- --txt-suffix={{ .Values.txtSuffix }}
{{- end}}

Copy link
Member Author

Choose a reason for hiding this comment

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

Make sense. Refactored and added test

- --txt-suffix={{ .Values.txtSuffix }}
{{- end }}
{{- if .Values.namespaced }}
Expand Down
6 changes: 6 additions & 0 deletions charts/external-dns/templates/validation.tpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{{/*
Mutually exclusive txtPrefix and txtSuffix
*/}}
{{- if and .Values.txtPrefix .Values.txtSuffix -}}
{{- fail (printf "'txtPrefix' and 'txtSuffix' mutually exclusive") -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a new pattern in the chart and I think if we're going to fail this needs to be part of the template to make it as easily understandable as possible. See above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed validation.tpl

{{- end -}}
11 changes: 10 additions & 1 deletion charts/external-dns/tests/deployment-flags_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ release:
tests:
- it: should provide expected default flags
asserts:
# - matchSnapshot: {}
- exists :
path: spec.template.spec.containers[?(@.name == "external-dns")]
- equal :
Expand Down Expand Up @@ -71,6 +70,16 @@ tests:
path: spec.template.spec.containers[?(@.name == "external-dns")].args
content: "--txt-prefix=test-prefix"

- it: should configure 'txtSuffix' when set and 'txtPrefix' is not present
set:
txtSuffix: "custom-suffix"
asserts:
- exists :
path: spec.template.spec.containers[?(@.name == "external-dns")]
- contains:
path: spec.template.spec.containers[?(@.name == "external-dns")].args
content: "--txt-suffix=custom-suffix"

- it: should be able configure multiple sources
set:
sources:
Expand Down
37 changes: 37 additions & 0 deletions charts/external-dns/tests/validation_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
suite: Test team namespace constraints
templates:
- validation.tpl
tests:
- it: should fail when txtPrefix and txtSuffix are set
templates:
- validation.tpl
set:
txtPrefix: "test-prefix"
txtSuffix: "test-suffix"
txtOwnerId: "testing"
asserts:
- failedTemplate:
errorMessage: "'txtPrefix' and 'txtSuffix' mutually exclusive"

- it: should not fail when txtPrefix is set
templates:
- validation.tpl
set:
txtPrefix: "test-prefix"
txtSuffix: ""
asserts:
- notFailedTemplate: {}

- it: should not fail when txtSuffix is set
templates:
- validation.tpl
set:
txtSuffix: "abrakadabra"
asserts:
- notFailedTemplate: {}

- it: should not fail with default values
templates:
- validation.tpl
asserts:
- notFailedTemplate: {}
Loading