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

Add annotation on config change #1001

Merged
merged 6 commits into from
Mar 18, 2024
Merged

Add annotation on config change #1001

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
16dd8e3
Add annotation on config change
Mar 7, 2024
f77fc3c
Address PR comments
Mar 11, 2024
fc5721d
Update templates/_helpers.tpl
swenson Mar 11, 2024
de1ca09
Update annotations
Mar 11, 2024
de9f647
Use name includeConfigAnnotation
Mar 12, 2024
fd5abeb
Check that annotations are not set by default
Mar 18, 2024
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
32 changes: 30 additions & 2 deletions templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,12 @@ Sets the injector deployment update strategy
{{/*
Sets extra pod annotations
*/}}
{{- define "vault.annotations" -}}
{{- if .Values.server.annotations }}
{{- define "vault.annotations" }}
annotations:
{{- if .Values.server.configAnnotation }}
vault.hashicorp.com/config-checksum: {{ include "vault.config" . | sha256sum }}
{{- end }}
{{- if .Values.server.annotations }}
{{- $tp := typeOf .Values.server.annotations }}
{{- if eq $tp "string" }}
{{- tpl .Values.server.annotations . | nindent 8 }}
Expand Down Expand Up @@ -1075,3 +1078,28 @@ Supported inputs are Values.ui
{{- end -}}
{{- end }}
{{- end -}}

{{/*
config file from values
*/}}
{{- define "vault.config" -}}
{{- if or (eq .mode "ha") (eq .mode "standalone") }}
{{- $type := typeOf (index .Values.server .mode).config }}
{{- if eq $type "string" }}
disable_mlock = true
{{- if eq .mode "standalone" }}
{{ tpl .Values.server.standalone.config . | nindent 4 | trim }}
{{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "false") }}
{{ tpl .Values.server.ha.config . | nindent 4 | trim }}
{{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
{{ tpl .Values.server.ha.raft.config . | nindent 4 | trim }}
{{ end }}
{{- else }}
{{- if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
{{ merge (dict "disable_mlock" true) (index .Values.server .mode).raft.config | toPrettyJson | indent 4 }}
{{- else }}
{{ merge (dict "disable_mlock" true) (index .Values.server .mode).config | toPrettyJson | indent 4 }}
{{- end }}
{{- end }}
{{- end }}
{{- end -}}
24 changes: 5 additions & 19 deletions templates/server-config-configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,13 @@ metadata:
app.kubernetes.io/name: {{ include "vault.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- if .Values.server.configAnnotation }}
annotations:
vault.hashicorp.com/config-checksum: {{ include "vault.config" . | sha256sum }}
{{- end }}
data:
extraconfig-from-values.hcl: |-
{{- if or (eq .mode "ha") (eq .mode "standalone") }}
{{- $type := typeOf (index .Values.server .mode).config }}
{{- if eq $type "string" }}
disable_mlock = true
{{- if eq .mode "standalone" }}
{{ tpl .Values.server.standalone.config . | nindent 4 | trim }}
{{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "false") }}
{{ tpl .Values.server.ha.config . | nindent 4 | trim }}
{{- else if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
{{ tpl .Values.server.ha.raft.config . | nindent 4 | trim }}
{{ end }}
{{- else }}
{{- if and (eq .mode "ha") (eq (.Values.server.ha.raft.enabled | toString) "true") }}
{{ merge (dict "disable_mlock" true) (index .Values.server .mode).raft.config | toPrettyJson | indent 4 }}
{{- else }}
{{ merge (dict "disable_mlock" true) (index .Values.server .mode).config | toPrettyJson | indent 4 }}
{{- end }}
{{- end }}
{{- end }}
{{ template "vault.config" . }}
{{- end }}
{{- end }}
{{- end }}
Expand Down
6 changes: 5 additions & 1 deletion test/docker/Test.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ RUN apk update && apk add --no-cache --virtual .build-deps \
jq

# yq
RUN pip install yq
RUN python3 -m venv venv && \
. venv/bin/activate && \
pip install yq && \
ln -s $PWD/venv/bin/yq /usr/local/bin/yq && \
deactivate

# gcloud
RUN curl -OL https://dl.google.com/dl/cloudsdk/channels/rapid/install_google_cloud_sdk.bash && \
Expand Down
19 changes: 19 additions & 0 deletions test/unit/server-configmap.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,22 @@ load _helpers
yq 'length > 0' | tee /dev/stderr)
[ "${actual}" = "false" ]
}

@test "server/ConfigMap: config checksum annotation defaults to off" {
cd `chart_dir`
local actual=$(helm template \
--show-only templates/server-config-configmap.yaml \
. | tee /dev/stderr |
yq '.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/ConfigMap: config checksum annotation can be enabled" {
cd `chart_dir`
local actual=$(helm template \
--show-only templates/server-config-configmap.yaml \
--set 'server.configAnnotation=true' \
. | tee /dev/stderr |
yq '.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr)
[ "${actual}" = "false" ]
}
19 changes: 19 additions & 0 deletions test/unit/server-statefulset.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,25 @@ load _helpers
[ "${actual}" = "true" ]
}

@test "server/standalone-StatefulSet: config checksum annotation defaults to off" {
cd `chart_dir`
local actual=$(helm template \
--show-only templates/server-statefulset.yaml \
. | tee /dev/stderr |
yq '.spec.template.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr)
[ "${actual}" = "true" ]
}

@test "server/standalone-StatefulSet: config checksum annotation can be enabled" {
cd `chart_dir`
local actual=$(helm template \
--show-only templates/server-statefulset.yaml \
--set 'server.configAnnotation=true' \
. | tee /dev/stderr |
yq '.spec.template.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr)
[ "${actual}" = "false" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also test the value here? Maybe assert something about its length, or expect a specific hash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that initially, but realized that the value might be something noisy and annoying to fix every time we change the config file.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are doing those checks as part of the other tests for this kind of feature:

vault-helm/test/unit/prometheus-servicemonitor.bats

Lines 84 to 85 in 0407431

[ "$(echo "$output" | yq -r '.metadata.labels | length')" = "5" ]
[ "$(echo "$output" | yq -r '.metadata.labels.release')" = "prometheus" ]
.

I think it is worth it to catch any potential regression where we could the expected defaults.

}

#--------------------------------------------------------------------
# priorityClassName

Expand Down
9 changes: 8 additions & 1 deletion values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,13 @@ server:
# of the annotations to apply to the server pods
annotations: {}

# Add an annotation to the server configmap and the statefulset pods,
# vaultproject.io/config-checksum, that is a hash of the Vault configuration.
# This can be used together with an OnDelete deployment strategy to help
# identify which pods still need to be deleted during a deployment to pick up
# any configuration changes.
configAnnotation: false
benashz marked this conversation as resolved.
Show resolved Hide resolved

# Enables a headless service to be used by the Vault Statefulset
service:
enabled: true
Expand Down Expand Up @@ -714,7 +721,7 @@ server:
# PreferDualStack: Allocates IPv4 and IPv6 cluster IPs for the Service.
# RequireDualStack: Allocates Service .spec.ClusterIPs from both IPv4 and IPv6 address ranges.
ipFamilyPolicy: ""

# Sets the families that should be supported and the order in which they should be applied to ClusterIP as well.
# Can be IPv4 and/or IPv6.
ipFamilies: []
Expand Down
Loading