From 27026e4657401502d8898feeb76ccee45294e42b Mon Sep 17 00:00:00 2001 From: RuriRyan <2413490+RuriRyan@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:15:55 +0200 Subject: [PATCH] fix(spire): preserve webhook order in patch hooks (#918) * fix(spire-server): preserve webhook order in patch hooks Signed-off-by: Christoph Manns <2413490+RuriRyan@users.noreply.github.com> * test(spire): cover webhook patch order Signed-off-by: Christoph Manns <2413490+RuriRyan@users.noreply.github.com> * fix(spire-server): honor effective hook setting Signed-off-by: Christoph Manns <2413490+RuriRyan@users.noreply.github.com> * Apply suggestion from @kfox1111 Signed-off-by: kfox1111 --------- Signed-off-by: Christoph Manns <2413490+RuriRyan@users.noreply.github.com> Signed-off-by: kfox1111 Signed-off-by: kfox1111 Co-authored-by: kfox1111 Co-authored-by: kfox1111 --- .../templates/controller-manager-webhook.yaml | 7 +- .../templates/post-install-hook.yaml | 4 +- .../templates/post-upgrade-hook.yaml | 4 +- .../templates/pre-upgrade-hook.yaml | 4 +- tests/unit/spire_test.go | 147 ++++++++++++++++++ 5 files changed, 159 insertions(+), 7 deletions(-) diff --git a/charts/spire/charts/spire-server/templates/controller-manager-webhook.yaml b/charts/spire/charts/spire-server/templates/controller-manager-webhook.yaml index 8e80679..5bb5758 100644 --- a/charts/spire/charts/spire-server/templates/controller-manager-webhook.yaml +++ b/charts/spire/charts/spire-server/templates/controller-manager-webhook.yaml @@ -1,3 +1,4 @@ +{{- $installAndUpgradeHooksEnabled := dig "installAndUpgradeHooks" "enabled" .Values.controllerManager.installAndUpgradeHook.enabled .Values.global }} {{- if not .Values.externalServer }} {{- if eq .Values.controllerManager.staticManifestMode "off" }} {{- if and (eq (.Values.controllerManager.enabled | toString) "true") .Values.controllerManager.validatingWebhookConfiguration.enabled }} @@ -12,7 +13,7 @@ webhooks: name: {{ include "spire-controller-manager.fullname" . }}-webhook namespace: {{ include "spire-server.namespace" . }} path: /validate-spire-spiffe-io-v1alpha1-clusterfederatedtrustdomain - {{- if eq (.Values.controllerManager.installAndUpgradeHook.enabled | toString) "true" }} + {{- if eq ($installAndUpgradeHooksEnabled | toString) "true" }} failurePolicy: Ignore # Actual value to be set by post install/upgrade hooks {{- else }} failurePolicy: {{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }} @@ -30,7 +31,11 @@ webhooks: name: {{ include "spire-controller-manager.fullname" . }}-webhook namespace: {{ include "spire-server.namespace" . }} path: /validate-spire-spiffe-io-v1alpha1-clusterspiffeid + {{- if eq ($installAndUpgradeHooksEnabled | toString) "true" }} failurePolicy: Ignore # Actual value to be set by post install/upgrade hooks + {{- else }} + failurePolicy: {{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }} + {{- end }} name: vclusterspiffeid.kb.io rules: - apiGroups: ["spire.spiffe.io"] diff --git a/charts/spire/charts/spire-server/templates/post-install-hook.yaml b/charts/spire/charts/spire-server/templates/post-install-hook.yaml index ed00051..ef0026e 100644 --- a/charts/spire/charts/spire-server/templates/post-install-hook.yaml +++ b/charts/spire/charts/spire-server/templates/post-install-hook.yaml @@ -85,11 +85,11 @@ spec: { "webhooks":[ { - "name":"vclusterspiffeid.kb.io", + "name":"vclusterfederatedtrustdomain.kb.io", "failurePolicy":"{{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }}" }, { - "name":"vclusterfederatedtrustdomain.kb.io", + "name":"vclusterspiffeid.kb.io", "failurePolicy":"{{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }}" } ] diff --git a/charts/spire/charts/spire-server/templates/post-upgrade-hook.yaml b/charts/spire/charts/spire-server/templates/post-upgrade-hook.yaml index fb42cfb..4fc2274 100644 --- a/charts/spire/charts/spire-server/templates/post-upgrade-hook.yaml +++ b/charts/spire/charts/spire-server/templates/post-upgrade-hook.yaml @@ -85,11 +85,11 @@ spec: { "webhooks":[ { - "name":"vclusterspiffeid.kb.io", + "name":"vclusterfederatedtrustdomain.kb.io", "failurePolicy":"{{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }}" }, { - "name":"vclusterfederatedtrustdomain.kb.io", + "name":"vclusterspiffeid.kb.io", "failurePolicy":"{{ .Values.controllerManager.validatingWebhookConfiguration.failurePolicy }}" } ] diff --git a/charts/spire/charts/spire-server/templates/pre-upgrade-hook.yaml b/charts/spire/charts/spire-server/templates/pre-upgrade-hook.yaml index 18a5bc6..f5f94e6 100644 --- a/charts/spire/charts/spire-server/templates/pre-upgrade-hook.yaml +++ b/charts/spire/charts/spire-server/templates/pre-upgrade-hook.yaml @@ -85,11 +85,11 @@ spec: { "webhooks":[ { - "name":"vclusterspiffeid.kb.io", + "name":"vclusterfederatedtrustdomain.kb.io", "failurePolicy":"Ignore" }, { - "name":"vclusterfederatedtrustdomain.kb.io", + "name":"vclusterspiffeid.kb.io", "failurePolicy":"Ignore" } ] diff --git a/tests/unit/spire_test.go b/tests/unit/spire_test.go index e76c249..4f11873 100644 --- a/tests/unit/spire_test.go +++ b/tests/unit/spire_test.go @@ -1,6 +1,10 @@ package unit_test import ( + "encoding/json" + "io" + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -8,8 +12,71 @@ import ( helmloader "helm.sh/helm/v3/pkg/chart/loader" helmutil "helm.sh/helm/v3/pkg/chartutil" helmengine "helm.sh/helm/v3/pkg/engine" + yamlutil "k8s.io/apimachinery/pkg/util/yaml" ) +type renderedWebhook struct { + Name string `json:"name"` + FailurePolicy string `json:"failurePolicy"` +} + +type renderedDocument struct { + Kind string `json:"kind"` + Metadata struct { + Annotations map[string]string `json:"annotations"` + } `json:"metadata"` + Spec struct { + Template struct { + Spec struct { + Containers []struct { + Args []string `json:"args"` + } `json:"containers"` + } `json:"spec"` + } `json:"template"` + } `json:"spec"` + Webhooks []renderedWebhook `json:"webhooks"` +} + +func decodeRenderedDocuments(rendered string) ([]renderedDocument, error) { + decoder := yamlutil.NewYAMLOrJSONDecoder(strings.NewReader(rendered), 4096) + var documents []renderedDocument + for { + var document renderedDocument + err := decoder.Decode(&document) + if err == io.EOF { + return documents, nil + } + if err != nil { + return nil, err + } + if document.Kind != "" { + documents = append(documents, document) + } + } +} + +func patchWebhookNames(job renderedDocument) ([]string, error) { + for _, container := range job.Spec.Template.Spec.Containers { + for index, arg := range container.Args { + if arg != "-p" || index+1 >= len(container.Args) { + continue + } + var patch struct { + Webhooks []renderedWebhook `json:"webhooks"` + } + if err := json.Unmarshal([]byte(container.Args[index+1]), &patch); err != nil { + return nil, err + } + names := make([]string, 0, len(patch.Webhooks)) + for _, webhook := range patch.Webhooks { + names = append(names, webhook.Name) + } + return names, nil + } + } + return nil, nil +} + func ValueStringRender(chart *helmchart.Chart, values string) (map[string]string, error) { v, err := helmutil.ReadValues([]byte(values)) if err != nil { @@ -663,6 +730,86 @@ spire-server: Expect(err).Should(Succeed()) }) }) + Describe("spire-server webhook patch order", func() { + It("preserves the rendered webhook order in every strategic-merge hook", func() { + objs, err := ValueStringRender(chart, ` +global: + installAndUpgradeHooks: + enabled: true +spire-server: + enabled: true + controllerManager: + enabled: true +`) + Expect(err).Should(Succeed()) + + canonicalDocuments, err := decodeRenderedDocuments(objs["spire/charts/spire-server/templates/controller-manager-webhook.yaml"]) + Expect(err).Should(Succeed()) + Expect(canonicalDocuments).Should(HaveLen(1)) + Expect(canonicalDocuments[0].Kind).Should(Equal("ValidatingWebhookConfiguration")) + canonicalNames := make([]string, 0, len(canonicalDocuments[0].Webhooks)) + for _, webhook := range canonicalDocuments[0].Webhooks { + canonicalNames = append(canonicalNames, webhook.Name) + } + + for _, hook := range []struct { + name string + template string + }{ + {name: "post-install", template: "spire/charts/spire-server/templates/post-install-hook.yaml"}, + {name: "pre-upgrade", template: "spire/charts/spire-server/templates/pre-upgrade-hook.yaml"}, + {name: "post-upgrade", template: "spire/charts/spire-server/templates/post-upgrade-hook.yaml"}, + } { + documents, err := decodeRenderedDocuments(objs[hook.template]) + Expect(err).Should(Succeed()) + var jobs []renderedDocument + for _, document := range documents { + if document.Kind == "Job" && document.Metadata.Annotations["helm.sh/hook"] == hook.name { + jobs = append(jobs, document) + } + } + Expect(jobs).Should(HaveLen(1), hook.name) + actualNames, err := patchWebhookNames(jobs[0]) + Expect(err).Should(Succeed()) + Expect(actualNames).Should(Equal(canonicalNames), hook.name) + } + }) + }) + Describe("spire-server webhook hooks disabled", func() { + It("uses the configured failure policy and omits lifecycle Jobs", func() { + objs, err := ValueStringRender(chart, ` +global: + installAndUpgradeHooks: + enabled: false +spire-server: + controllerManager: + enabled: true + validatingWebhookConfiguration: + failurePolicy: Fail +`) + Expect(err).Should(Succeed()) + + canonicalDocuments, err := decodeRenderedDocuments(objs["spire/charts/spire-server/templates/controller-manager-webhook.yaml"]) + Expect(err).Should(Succeed()) + Expect(canonicalDocuments).Should(HaveLen(1)) + Expect(canonicalDocuments[0].Webhooks).Should(HaveLen(2)) + for _, webhook := range canonicalDocuments[0].Webhooks { + Expect(webhook.FailurePolicy).Should(Equal("Fail")) + } + + for _, template := range []string{ + "spire/charts/spire-server/templates/post-install-hook.yaml", + "spire/charts/spire-server/templates/pre-upgrade-hook.yaml", + "spire/charts/spire-server/templates/post-upgrade-hook.yaml", + } { + documents, err := decodeRenderedDocuments(objs[template]) + Expect(err).Should(Succeed()) + for _, document := range documents { + Expect(document.Kind).ShouldNot(Equal("Job"), template) + } + } + }) + }) Describe("spire-server.dataStore.sql.postgres passwordless", func() { It("omits password and the -dbpw Secret for cert auth with an empty password", func() { objs, err := ValueStringRender(chart, `