Skip to content

Commit cdfb8b0

Browse files
camilamacedo86camilamacedo86
committed
(helm/v1alpha1) - fix webhook generation by removing data from helm chart values
This commit changes the code implementation so that the webhook values in the helm chart are not generated. Instead, only expose the values to enable or not webhooks
1 parent 980fb71 commit cdfb8b0

File tree

8 files changed

+116
-314
lines changed

8 files changed

+116
-314
lines changed

‎pkg/plugins/optional/helm/v1alpha/scaffolds/init.go‎

Lines changed: 49 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ import (
3232
"sigs.k8s.io/kubebuilder/v4/pkg/plugin"
3333
"sigs.k8s.io/kubebuilder/v4/pkg/plugins"
3434
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/golang/deploy-image/v1alpha1"
35-
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm"
3635
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates"
37-
chart_templates "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates"
36+
charttemplates "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates"
3837
templatescertmanager "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/cert-manager"
3938
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/manager"
4039
templatesmetrics "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/metrics"
@@ -70,11 +69,9 @@ func (s *initScaffolder) InjectFS(fs machinery.Filesystem) {
7069
func (s *initScaffolder) Scaffold() error {
7170
log.Println("Generating Helm Chart to distribute project")
7271

73-
// Extract Images scaffolded with DeployImage to add ENVVAR to the values
7472
imagesEnvVars := s.getDeployImagesEnvVars()
7573

76-
// Extract webhooks from generated YAML files (generated by controller-gen)
77-
webhooks, err := extractWebhooksFromGeneratedFiles()
74+
webhooks, err := s.extractWebhooksFromGeneratedFiles()
7875
if err != nil {
7976
return fmt.Errorf("failed to extract webhooks: %w", err)
8077
}
@@ -88,12 +85,11 @@ func (s *initScaffolder) Scaffold() error {
8885
&templates.HelmChart{},
8986
&templates.HelmValues{
9087
HasWebhooks: len(webhooks) > 0,
91-
Webhooks: webhooks,
9288
DeployImages: imagesEnvVars,
9389
Force: s.force,
9490
},
9591
&templates.HelmIgnore{},
96-
&chart_templates.HelmHelpers{},
92+
&charttemplates.HelmHelpers{},
9793
&manager.ManagerDeployment{
9894
Force: s.force,
9995
DeployImages: len(imagesEnvVars) > 0,
@@ -105,8 +101,12 @@ func (s *initScaffolder) Scaffold() error {
105101
}
106102

107103
if len(webhooks) > 0 {
108-
buildScaffold = append(buildScaffold, &templateswebhooks.WebhookTemplate{})
109-
buildScaffold = append(buildScaffold, &templateswebhooks.WebhookService{})
104+
buildScaffold = append(buildScaffold,
105+
&templateswebhooks.WebhookTemplate{
106+
Webhooks: webhooks,
107+
},
108+
&templateswebhooks.WebhookService{},
109+
)
110110
}
111111

112112
if err := scaffold.Execute(buildScaffold...); err != nil {
@@ -146,87 +146,73 @@ func (s *initScaffolder) getDeployImagesEnvVars() map[string]string {
146146
return deployImages
147147
}
148148

149-
// Extract webhooks from manifests.yaml file
150-
func extractWebhooksFromGeneratedFiles() ([]helm.WebhookYAML, error) {
151-
var webhooks []helm.WebhookYAML
149+
// extractWebhooksFromGeneratedFiles parse the files and get the webhook data
150+
func (s *initScaffolder) extractWebhooksFromGeneratedFiles() ([]templateswebhooks.Webhook, error) {
151+
var webhooks []templateswebhooks.Webhook
152152
manifestFile := "config/webhook/manifests.yaml"
153-
if _, err := os.Stat(manifestFile); err == nil {
154-
content, err := os.ReadFile(manifestFile)
155-
if err != nil {
156-
return nil, fmt.Errorf("failed to read manifests.yaml: %w", err)
157-
}
158153

159-
// Process the content to extract webhooks
160-
webhooks = append(webhooks, extractWebhookYAML(content)...)
161-
} else {
162-
// Return empty if no webhooks were found
154+
if _, err := os.Stat(manifestFile); os.IsNotExist(err) {
155+
log.Printf("webhook manifests were not found at %s", manifestFile)
163156
return webhooks, nil
164157
}
165158

166-
return webhooks, nil
167-
}
168-
169-
// extractWebhookYAML parses the webhook YAML content and returns a list of WebhookYAML
170-
func extractWebhookYAML(content []byte) []helm.WebhookYAML {
171-
var webhooks []helm.WebhookYAML
172-
173-
type WebhookConfig struct {
174-
Kind string `yaml:"kind"`
175-
Webhooks []struct {
176-
Name string `yaml:"name"`
177-
ClientConfig struct {
178-
Service struct {
179-
Name string `yaml:"name"`
180-
Namespace string `yaml:"namespace"`
181-
Path string `yaml:"path"`
182-
} `yaml:"service"`
183-
CABundle string `yaml:"caBundle"`
184-
} `yaml:"clientConfig"`
185-
Rules []helm.WebhookRule `yaml:"rules"`
186-
FailurePolicy string `yaml:"failurePolicy"`
187-
SideEffects string `yaml:"sideEffects"`
188-
AdmissionReviewVersions []string `yaml:"admissionReviewVersions"`
189-
} `yaml:"webhooks"`
159+
content, err := os.ReadFile(manifestFile)
160+
if err != nil {
161+
return nil, fmt.Errorf("failed to read %s: %w", manifestFile, err)
190162
}
191163

192-
// Split the input into different documents (to handle multiple YAML docs in one file)
193164
docs := strings.Split(string(content), "---")
194-
195165
for _, doc := range docs {
196-
var webhookConfig WebhookConfig
166+
var webhookConfig struct {
167+
Kind string `yaml:"kind"`
168+
Webhooks []struct {
169+
Name string `yaml:"name"`
170+
ClientConfig struct {
171+
Service struct {
172+
Name string `yaml:"name"`
173+
Namespace string `yaml:"namespace"`
174+
Path string `yaml:"path"`
175+
} `yaml:"service"`
176+
} `yaml:"clientConfig"`
177+
Rules []templateswebhooks.WebhookRule `yaml:"rules"`
178+
FailurePolicy string `yaml:"failurePolicy"`
179+
SideEffects string `yaml:"sideEffects"`
180+
AdmissionReviewVersions []string `yaml:"admissionReviewVersions"`
181+
} `yaml:"webhooks"`
182+
}
183+
197184
if err := yaml.Unmarshal([]byte(doc), &webhookConfig); err != nil {
198185
log.Errorf("Error unmarshalling webhook YAML: %v", err)
199186
continue
200187
}
201188

202-
// Determine the webhook type (mutating or validating)
203189
webhookType := "unknown"
204190
if webhookConfig.Kind == "MutatingWebhookConfiguration" {
205191
webhookType = "mutating"
206192
} else if webhookConfig.Kind == "ValidatingWebhookConfiguration" {
207193
webhookType = "validating"
208194
}
209195

210-
// Parse each webhook and append it to the result
211-
for _, webhook := range webhookConfig.Webhooks {
212-
for i := range webhook.Rules {
213-
// If apiGroups is empty, set it to [""] to ensure proper YAML output
214-
if len(webhook.Rules[i].APIGroups) == 0 {
215-
webhook.Rules[i].APIGroups = []string{""}
196+
for _, w := range webhookConfig.Webhooks {
197+
for i := range w.Rules {
198+
if len(w.Rules[i].APIGroups) == 0 {
199+
w.Rules[i].APIGroups = []string{""}
216200
}
217201
}
218-
webhooks = append(webhooks, helm.WebhookYAML{
219-
Name: webhook.Name,
202+
webhooks = append(webhooks, templateswebhooks.Webhook{
203+
NameMetadata: fmt.Sprintf("%s-%s-webhook-configuration", s.config.GetProjectName(), webhookType),
204+
Name: w.Name,
220205
Type: webhookType,
221-
Path: webhook.ClientConfig.Service.Path,
222-
Rules: webhook.Rules,
223-
FailurePolicy: webhook.FailurePolicy,
224-
SideEffects: webhook.SideEffects,
225-
AdmissionReviewVersions: webhook.AdmissionReviewVersions,
206+
ServiceName: fmt.Sprintf("%s-webhook-service", s.config.GetProjectName()),
207+
Path: w.ClientConfig.Service.Path,
208+
FailurePolicy: w.FailurePolicy,
209+
SideEffects: w.SideEffects,
210+
AdmissionReviewVersions: w.AdmissionReviewVersions,
211+
Rules: w.Rules,
226212
})
227213
}
228214
}
229-
return webhooks
215+
return webhooks, nil
230216
}
231217

232218
// Helper function to copy files from config/ to dist/chart/templates/

‎pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/webhook/service.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ var _ machinery.Template = &WebhookService{}
2626
// WebhookService scaffolds the Service for webhooks in the Helm chart
2727
type WebhookService struct {
2828
machinery.TemplateMixin
29+
machinery.ProjectNameMixin
2930

3031
// Force if true allows overwriting the scaffolded file
3132
Force bool
@@ -48,7 +49,7 @@ const webhookServiceTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` +
4849
apiVersion: v1
4950
kind: Service
5051
metadata:
51-
name: {{ "{{ include \"chart.name\" . }}" }}-webhook-service
52+
name: {{ .ProjectName }}-webhook-service
5253
namespace: {{ "{{ .Release.Namespace }}" }}
5354
labels:
5455
{{ "{{- include \"chart.labels\" . | nindent 4 }}" }}

‎pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/webhook/webhook.go‎

Lines changed: 50 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ var _ machinery.Template = &WebhookTemplate{}
2727
type WebhookTemplate struct {
2828
machinery.TemplateMixin
2929
machinery.ProjectNameMixin
30+
31+
Webhooks []Webhook
3032
}
3133

3234
// SetTemplateDefaults sets default configuration for the webhook template
@@ -36,18 +38,37 @@ func (f *WebhookTemplate) SetTemplateDefaults() error {
3638
}
3739

3840
f.TemplateBody = webhookTemplate
39-
4041
f.IfExistsAction = machinery.OverwriteFile
41-
4242
return nil
4343
}
4444

45-
const webhookTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` + "`" + `}}
45+
// Webhook helps generate the bollerplate with the Webhook values
46+
type Webhook struct {
47+
ServiceName string
48+
NameMetadata string
49+
Name string
50+
Path string
51+
Type string
52+
FailurePolicy string
53+
SideEffects string
54+
AdmissionReviewVersions []string
55+
Rules []WebhookRule
56+
}
57+
58+
// WebhookRule to help map the rules
59+
type WebhookRule struct {
60+
Operations []string
61+
APIGroups []string
62+
APIVersions []string
63+
Resources []string
64+
}
4665

66+
const webhookTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` + "`" + `}}
67+
{{- range .Webhooks }}
4768
apiVersion: admissionregistration.k8s.io/v1
48-
kind: MutatingWebhookConfiguration
69+
kind: {{ if eq .Type "mutating" }}MutatingWebhookConfiguration{{ else }}ValidatingWebhookConfiguration{{ end }}
4970
metadata:
50-
name: {{ .ProjectName }}-mutating-webhook-configuration
71+
name: {{ .NameMetadata }}
5172
namespace: {{ "{{ .Release.Namespace }}" }}
5273
annotations:
5374
{{` + "`" + `{{- if .Values.certmanager.enable }}` + "`" + `}}
@@ -56,87 +77,38 @@ metadata:
5677
labels:
5778
{{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
5879
webhooks:
59-
{{` + "`" + `{{- range .Values.webhook.services }}` + "`" + `}}
60-
{{` + "`" + `{{- if eq .type "mutating" }}` + "`" + `}}
61-
- name: {{` + "`" + `{{ .name }}` + "`" + `}}
62-
clientConfig:
63-
service:
64-
name: {{ .ProjectName }}-webhook-service
65-
namespace: {{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}
66-
path: {{` + "`" + `{{ .path }}` + "`" + `}}
67-
failurePolicy: {{` + "`" + `{{ .failurePolicy }}` + "`" + `}}
68-
sideEffects: {{` + "`" + `{{ .sideEffects }}` + "`" + `}}
69-
admissionReviewVersions:
70-
{{` + "`" + `{{- range .admissionReviewVersions }}` + "`" + `}}
71-
- {{` + "`" + `{{ . }}` + "`" + `}}
72-
{{` + "`" + `{{- end }}` + "`" + `}}
73-
rules:
74-
{{` + "`" + `{{- range .rules }}` + "`" + `}}
75-
- operations:
76-
{{` + "`" + `{{- range .operations }}` + "`" + `}}
77-
- {{` + "`" + `{{ . }}` + "`" + `}}
78-
{{` + "`" + `{{- end }}` + "`" + `}}
79-
apiGroups:
80-
{{` + "`" + `{{- range .apiGroups }}` + "`" + `}}
81-
- {{` + "`" + `{{ . }}` + "`" + `}}
82-
{{` + "`" + `{{- end }}` + "`" + `}}
83-
apiVersions:
84-
{{` + "`" + `{{- range .apiVersions }}` + "`" + `}}
85-
- {{` + "`" + `{{ . }}` + "`" + `}}
86-
{{` + "`" + `{{- end }}` + "`" + `}}
87-
resources:
88-
{{` + "`" + `{{- range .resources }}` + "`" + `}}
89-
- {{` + "`" + `{{ . }}` + "`" + `}}
90-
{{` + "`" + `{{- end }}` + "`" + `}}
91-
{{` + "`" + `{{- end }}` + "`" + `}}
92-
{{` + "`" + `{{- end }}` + "`" + `}}
93-
{{` + "`" + `{{- end }}` + "`" + `}}
94-
---
95-
apiVersion: admissionregistration.k8s.io/v1
96-
kind: ValidatingWebhookConfiguration
97-
metadata:
98-
name: {{ .ProjectName }}-validating-webhook-configuration
99-
namespace: {{ "{{ .Release.Namespace }}" }}
100-
annotations:
101-
{{` + "`" + `{{- if .Values.certmanager.enable }}` + "`" + `}}
102-
cert-manager.io/inject-ca-from: "{{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}/serving-cert"
103-
{{` + "`" + `{{- end }}` + "`" + `}}
104-
webhooks:
105-
{{` + "`" + `{{- range .Values.webhook.services }}` + "`" + `}}
106-
{{` + "`" + `{{- if eq .type "validating" }}` + "`" + `}}
107-
- name: {{` + "`" + `{{ .name }}` + "`" + `}}
80+
- name: {{ .Name}}
10881
clientConfig:
10982
service:
110-
name: {{ .ProjectName }}-webhook-service
111-
namespace: {{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}
112-
path: {{` + "`" + `{{ .path }}` + "`" + `}}
113-
failurePolicy: {{` + "`" + `{{ .failurePolicy }}` + "`" + `}}
114-
sideEffects: {{` + "`" + `{{ .sideEffects }}` + "`" + `}}
83+
name: {{ .ServiceName }}
84+
namespace: {{ "{{ .Release.Namespace }}" }}
85+
path: {{ .Path }}
86+
failurePolicy: {{ .FailurePolicy }}
87+
sideEffects: {{ .SideEffects }}
11588
admissionReviewVersions:
116-
{{` + "`" + `{{- range .admissionReviewVersions }}` + "`" + `}}
117-
- {{` + "`" + `{{ . }}` + "`" + `}}
118-
{{` + "`" + `{{- end }}` + "`" + `}}
89+
{{- range .AdmissionReviewVersions }}
90+
- {{ . }}
91+
{{- end }}
11992
rules:
120-
{{` + "`" + `{{- range .rules }}` + "`" + `}}
93+
{{- range .Rules }}
12194
- operations:
122-
{{` + "`" + `{{- range .operations }}` + "`" + `}}
123-
- {{` + "`" + `{{ . }}` + "`" + `}}
124-
{{` + "`" + `{{- end }}` + "`" + `}}
95+
{{- range .Operations }}
96+
- {{ . }}
97+
{{- end }}
12598
apiGroups:
126-
{{` + "`" + `{{- range .apiGroups }}` + "`" + `}}
127-
- {{` + "`" + `{{ . }}` + "`" + `}}
128-
{{` + "`" + `{{- end }}` + "`" + `}}
99+
{{- range .APIGroups }}
100+
- {{ . }}
101+
{{- end }}
129102
apiVersions:
130-
{{` + "`" + `{{- range .apiVersions }}` + "`" + `}}
131-
- {{` + "`" + `{{ . }}` + "`" + `}}
132-
{{` + "`" + `{{- end }}` + "`" + `}}
103+
{{- range .APIVersions }}
104+
- {{ . }}
105+
{{- end }}
133106
resources:
134-
{{` + "`" + `{{- range .resources }}` + "`" + `}}
135-
- {{` + "`" + `{{ . }}` + "`" + `}}
136-
{{` + "`" + `{{- end }}` + "`" + `}}
137-
{{` + "`" + `{{- end }}` + "`" + `}}
138-
{{` + "`" + `{{- end }}` + "`" + `}}
139-
{{` + "`" + `{{- end }}` + "`" + `}}
107+
{{- range .Resources }}
108+
- {{ . }}
109+
{{- end }}
110+
{{- end }}
140111
---
112+
{{- end }}
141113
{{` + "`" + `{{- end }}` + "`" + `}}
142114
`

0 commit comments

Comments
 (0)