Skip to content

Commit

Permalink
(helm/v1alpha1) - fix webhook generation by removing data from helm c…
Browse files Browse the repository at this point in the history
…hart 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
  • Loading branch information
camilamacedo86 committed Dec 30, 2024
1 parent a8e72c3 commit bbc1bd9
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 314 deletions.
112 changes: 49 additions & 63 deletions pkg/plugins/optional/helm/v1alpha/scaffolds/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ import (
"sigs.k8s.io/kubebuilder/v4/pkg/plugin"
"sigs.k8s.io/kubebuilder/v4/pkg/plugins"
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/golang/deploy-image/v1alpha1"
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm"
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates"
chart_templates "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates"
chartTemplates "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates"
templatescertmanager "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/cert-manager"
"sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/manager"
templatesmetrics "sigs.k8s.io/kubebuilder/v4/pkg/plugins/optional/helm/v1alpha/scaffolds/internal/templates/chart-templates/metrics"
Expand Down Expand Up @@ -70,11 +69,9 @@ func (s *initScaffolder) InjectFS(fs machinery.Filesystem) {
func (s *initScaffolder) Scaffold() error {
log.Println("Generating Helm Chart to distribute project")

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

// Extract webhooks from generated YAML files (generated by controller-gen)
webhooks, err := extractWebhooksFromGeneratedFiles()
webhooks, err := s.extractWebhooksFromGeneratedFiles()
if err != nil {
return fmt.Errorf("failed to extract webhooks: %w", err)
}
Expand All @@ -88,12 +85,11 @@ func (s *initScaffolder) Scaffold() error {
&templates.HelmChart{},
&templates.HelmValues{
HasWebhooks: len(webhooks) > 0,
Webhooks: webhooks,
DeployImages: imagesEnvVars,
Force: s.force,
},
&templates.HelmIgnore{},
&chart_templates.HelmHelpers{},
&chartTemplates.HelmHelpers{},
&manager.ManagerDeployment{
Force: s.force,
DeployImages: len(imagesEnvVars) > 0,
Expand All @@ -105,8 +101,12 @@ func (s *initScaffolder) Scaffold() error {
}

if len(webhooks) > 0 {
buildScaffold = append(buildScaffold, &templateswebhooks.WebhookTemplate{})
buildScaffold = append(buildScaffold, &templateswebhooks.WebhookService{})
buildScaffold = append(buildScaffold,
&templateswebhooks.WebhookTemplate{
Webhooks: webhooks,
},
&templateswebhooks.WebhookService{},
)
}

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

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

// Process the content to extract webhooks
webhooks = append(webhooks, extractWebhookYAML(content)...)
} else {
// Return empty if no webhooks were found
if _, err := os.Stat(manifestFile); os.IsNotExist(err) {
log.Printf("webhook manifests were not found at %s", manifestFile)
return webhooks, nil
}

return webhooks, nil
}

// extractWebhookYAML parses the webhook YAML content and returns a list of WebhookYAML
func extractWebhookYAML(content []byte) []helm.WebhookYAML {
var webhooks []helm.WebhookYAML

type WebhookConfig struct {
Kind string `yaml:"kind"`
Webhooks []struct {
Name string `yaml:"name"`
ClientConfig struct {
Service struct {
Name string `yaml:"name"`
Namespace string `yaml:"namespace"`
Path string `yaml:"path"`
} `yaml:"service"`
CABundle string `yaml:"caBundle"`
} `yaml:"clientConfig"`
Rules []helm.WebhookRule `yaml:"rules"`
FailurePolicy string `yaml:"failurePolicy"`
SideEffects string `yaml:"sideEffects"`
AdmissionReviewVersions []string `yaml:"admissionReviewVersions"`
} `yaml:"webhooks"`
content, err := os.ReadFile(manifestFile)
if err != nil {
return nil, fmt.Errorf("failed to read %s: %w", manifestFile, err)
}

// Split the input into different documents (to handle multiple YAML docs in one file)
docs := strings.Split(string(content), "---")

for _, doc := range docs {
var webhookConfig WebhookConfig
var webhookConfig struct {
Kind string `yaml:"kind"`
Webhooks []struct {
Name string `yaml:"name"`
ClientConfig struct {
Service struct {
Name string `yaml:"name"`
Namespace string `yaml:"namespace"`
Path string `yaml:"path"`
} `yaml:"service"`
} `yaml:"clientConfig"`
Rules []templateswebhooks.WebhookRule `yaml:"rules"`
FailurePolicy string `yaml:"failurePolicy"`
SideEffects string `yaml:"sideEffects"`
AdmissionReviewVersions []string `yaml:"admissionReviewVersions"`
} `yaml:"webhooks"`
}

if err := yaml.Unmarshal([]byte(doc), &webhookConfig); err != nil {
log.Errorf("Error unmarshalling webhook YAML: %v", err)
continue
}

// Determine the webhook type (mutating or validating)
webhookType := "unknown"
if webhookConfig.Kind == "MutatingWebhookConfiguration" {
webhookType = "mutating"
} else if webhookConfig.Kind == "ValidatingWebhookConfiguration" {
webhookType = "validating"
}

// Parse each webhook and append it to the result
for _, webhook := range webhookConfig.Webhooks {
for i := range webhook.Rules {
// If apiGroups is empty, set it to [""] to ensure proper YAML output
if len(webhook.Rules[i].APIGroups) == 0 {
webhook.Rules[i].APIGroups = []string{""}
for _, w := range webhookConfig.Webhooks {
for i := range w.Rules {
if len(w.Rules[i].APIGroups) == 0 {
w.Rules[i].APIGroups = []string{""}
}
}
webhooks = append(webhooks, helm.WebhookYAML{
Name: webhook.Name,
webhooks = append(webhooks, templateswebhooks.Webhook{
NameMetadata: fmt.Sprintf("%s-%s-webhook-configuration", s.config.GetProjectName(), webhookType),
Name: w.Name,
Type: webhookType,
Path: webhook.ClientConfig.Service.Path,
Rules: webhook.Rules,
FailurePolicy: webhook.FailurePolicy,
SideEffects: webhook.SideEffects,
AdmissionReviewVersions: webhook.AdmissionReviewVersions,
ServiceName: fmt.Sprintf("%s-webhook-service", s.config.GetProjectName()),
Path: w.ClientConfig.Service.Path,
FailurePolicy: w.FailurePolicy,
SideEffects: w.SideEffects,
AdmissionReviewVersions: w.AdmissionReviewVersions,
Rules: w.Rules,
})
}
}
return webhooks
return webhooks, nil
}

// Helper function to copy files from config/ to dist/chart/templates/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var _ machinery.Template = &WebhookService{}
// WebhookService scaffolds the Service for webhooks in the Helm chart
type WebhookService struct {
machinery.TemplateMixin
machinery.ProjectNameMixin

// Force if true allows overwriting the scaffolded file
Force bool
Expand All @@ -48,7 +49,7 @@ const webhookServiceTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` +
apiVersion: v1
kind: Service
metadata:
name: {{ "{{ include \"chart.name\" . }}" }}-webhook-service
name: {{ .ProjectName }}-webhook-service
namespace: {{ "{{ .Release.Namespace }}" }}
labels:
{{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ var _ machinery.Template = &WebhookTemplate{}
type WebhookTemplate struct {
machinery.TemplateMixin
machinery.ProjectNameMixin

Webhooks []Webhook
}

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

f.TemplateBody = webhookTemplate

f.IfExistsAction = machinery.OverwriteFile

return nil
}

const webhookTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` + "`" + `}}
// Webhook helps generate the bollerplate with the Webhook values
type Webhook struct {
ServiceName string
NameMetadata string
Name string
Path string
Type string
FailurePolicy string
SideEffects string
AdmissionReviewVersions []string
Rules []WebhookRule
}

// WebhookRule to help map the rules
type WebhookRule struct {
Operations []string
APIGroups []string
APIVersions []string
Resources []string
}

const webhookTemplate = `{{` + "`" + `{{- if .Values.webhook.enable }}` + "`" + `}}
{{- range .Webhooks }}
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
kind: {{ if eq .Type "mutating" }}MutatingWebhookConfiguration{{ else }}ValidatingWebhookConfiguration{{ end }}
metadata:
name: {{ .ProjectName }}-mutating-webhook-configuration
name: {{ .NameMetadata }}
namespace: {{ "{{ .Release.Namespace }}" }}
annotations:
{{` + "`" + `{{- if .Values.certmanager.enable }}` + "`" + `}}
Expand All @@ -56,87 +77,38 @@ metadata:
labels:
{{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
webhooks:
{{` + "`" + `{{- range .Values.webhook.services }}` + "`" + `}}
{{` + "`" + `{{- if eq .type "mutating" }}` + "`" + `}}
- name: {{` + "`" + `{{ .name }}` + "`" + `}}
clientConfig:
service:
name: {{ .ProjectName }}-webhook-service
namespace: {{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}
path: {{` + "`" + `{{ .path }}` + "`" + `}}
failurePolicy: {{` + "`" + `{{ .failurePolicy }}` + "`" + `}}
sideEffects: {{` + "`" + `{{ .sideEffects }}` + "`" + `}}
admissionReviewVersions:
{{` + "`" + `{{- range .admissionReviewVersions }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
rules:
{{` + "`" + `{{- range .rules }}` + "`" + `}}
- operations:
{{` + "`" + `{{- range .operations }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
apiGroups:
{{` + "`" + `{{- range .apiGroups }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
apiVersions:
{{` + "`" + `{{- range .apiVersions }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
resources:
{{` + "`" + `{{- range .resources }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
name: {{ .ProjectName }}-validating-webhook-configuration
namespace: {{ "{{ .Release.Namespace }}" }}
annotations:
{{` + "`" + `{{- if .Values.certmanager.enable }}` + "`" + `}}
cert-manager.io/inject-ca-from: "{{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}/serving-cert"
{{` + "`" + `{{- end }}` + "`" + `}}
webhooks:
{{` + "`" + `{{- range .Values.webhook.services }}` + "`" + `}}
{{` + "`" + `{{- if eq .type "validating" }}` + "`" + `}}
- name: {{` + "`" + `{{ .name }}` + "`" + `}}
- name: {{ .Name}}
clientConfig:
service:
name: {{ .ProjectName }}-webhook-service
namespace: {{` + "`" + `{{ $.Release.Namespace }}` + "`" + `}}
path: {{` + "`" + `{{ .path }}` + "`" + `}}
failurePolicy: {{` + "`" + `{{ .failurePolicy }}` + "`" + `}}
sideEffects: {{` + "`" + `{{ .sideEffects }}` + "`" + `}}
name: {{ .ServiceName }}
namespace: {{ "{{ .Release.Namespace }}" }}
path: {{ .Path }}
failurePolicy: {{ .FailurePolicy }}
sideEffects: {{ .SideEffects }}
admissionReviewVersions:
{{` + "`" + `{{- range .admissionReviewVersions }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{- range .AdmissionReviewVersions }}
- {{ . }}
{{- end }}
rules:
{{` + "`" + `{{- range .rules }}` + "`" + `}}
{{- range .Rules }}
- operations:
{{` + "`" + `{{- range .operations }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{- range .Operations }}
- {{ . }}
{{- end }}
apiGroups:
{{` + "`" + `{{- range .apiGroups }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{- range .APIGroups }}
- {{ . }}
{{- end }}
apiVersions:
{{` + "`" + `{{- range .apiVersions }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{- range .APIVersions }}
- {{ . }}
{{- end }}
resources:
{{` + "`" + `{{- range .resources }}` + "`" + `}}
- {{` + "`" + `{{ . }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{` + "`" + `{{- end }}` + "`" + `}}
{{- range .Resources }}
- {{ . }}
{{- end }}
{{- end }}
---
{{- end }}
{{` + "`" + `{{- end }}` + "`" + `}}
`
Loading

0 comments on commit bbc1bd9

Please sign in to comment.