Skip to content

✨ Allow helm plugin to use custom prefixes #5119

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
+107 −15
Open

✨ Allow helm plugin to use custom prefixes #5119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4df61fe
feat: Allow helm templater plugin to get custom prefixes from kustomize
liam-mackie Oct 7, 2025
5b12a7c
Add tests
liam-mackie Oct 7, 2025
6dd6a78
Make kustomize file configurable
liam-mackie Oct 7, 2025
dcb24ca
Add NamePrefix to Service Monitor scaffold
liam-mackie Oct 8, 2025
4d79f65
Standardise to NamePrefix
liam-mackie Oct 8, 2025
44e16fe
Linting fixes
liam-mackie Oct 14, 2025
d2a79c6
Add service name from kustomize
liam-mackie Oct 14, 2025
182aa72
Get data from kustomize output rather than kustomization.yaml
liam-mackie Oct 14, 2025
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
21 changes: 18 additions & 3 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/edit_kustomize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ func (s *editKustomizeScaffolder) Scaffold() error {
slog.Warn("failed to remove stale generic ServiceMonitor", "path", staleSM, "error", rmErr)
}
}
chartConverter := kustomize.NewChartConverter(resources, s.config.GetProjectName(), s.outputDir)
namePrefix := resources.EstimatePrefix(s.config.GetProjectName())
chartConverter := kustomize.NewChartConverter(resources, s.config.GetProjectName(), namePrefix, s.outputDir)
deploymentConfig := chartConverter.ExtractDeploymentConfig()

// Create scaffold for standard Helm chart files
Expand All @@ -119,7 +120,8 @@ func (s *editKustomizeScaffolder) Scaffold() error {
chartFiles := []machinery.Builder{
&github.HelmChartCI{}, // GitHub Actions workflow for chart testing
&templates.HelmChart{OutputDir: s.outputDir}, // Chart.yaml metadata
&templates.HelmValuesBasic{ // values.yaml with dynamic config
&templates.HelmValuesBasic{
// values.yaml with dynamic config
HasWebhooks: hasWebhooks,
HasMetrics: hasMetrics,
DeploymentConfig: deploymentConfig,
Expand All @@ -134,7 +136,20 @@ func (s *editKustomizeScaffolder) Scaffold() error {
// provide one via kustomize (../prometheus). This avoids duplicate objects
// with the same name within the Helm chart.
if !hasPrometheus {
chartFiles = append(chartFiles, &charttemplates.ServiceMonitor{OutputDir: s.outputDir})
// Find the metrics service name from parsed resources
metricsServiceName := namePrefix + "-controller-manager-metrics-service"
for _, svc := range resources.Services {
if strings.Contains(svc.GetName(), "metrics-service") {
metricsServiceName = svc.GetName()
break
}
}

chartFiles = append(chartFiles, &charttemplates.ServiceMonitor{
OutputDir: s.outputDir,
NamePrefix: namePrefix,
ServiceName: metricsServiceName,
})
}

// Generate template files from kustomize output
Expand Down
4 changes: 2 additions & 2 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/chart_converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ type ChartConverter struct {
}

// NewChartConverter creates a new chart converter with all necessary components
func NewChartConverter(resources *ParsedResources, projectName, outputDir string) *ChartConverter {
func NewChartConverter(resources *ParsedResources, projectName, namePrefix, outputDir string) *ChartConverter {
organizer := NewResourceOrganizer(resources)
templater := NewHelmTemplater(projectName)
templater := NewHelmTemplater(projectName, namePrefix)
writer := NewChartWriter(templater, outputDir)

return &ChartConverter{
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/chart_converter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ var _ = Describe("ChartConverter", func() {
fs = machinery.Filesystem{FS: afero.NewMemMapFs()}

// Create converter
converter = NewChartConverter(resources, "test-project", "dist")
converter = NewChartConverter(resources, "test-project", "test-project", "dist")
})

Context("NewChartConverter", func() {
Expand Down
11 changes: 7 additions & 4 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/chart_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ func NewChartWriter(templater *HelmTemplater, outputDir string) *ChartWriter {
}

// WriteResourceGroup writes a group of resources to a Helm template file
func (w *ChartWriter) WriteResourceGroup(fs machinery.Filesystem, groupName string,
func (w *ChartWriter) WriteResourceGroup(
fs machinery.Filesystem, groupName string,
resources []*unstructured.Unstructured,
) error {
// Special handling for namespace - write as single file
Expand Down Expand Up @@ -73,7 +74,8 @@ func (w *ChartWriter) writeNamespaceFile(fs machinery.Filesystem, namespace *uns
}

// writeGroupDirectory writes resources as files in a group-specific directory
func (w *ChartWriter) writeGroupDirectory(fs machinery.Filesystem, groupName string,
func (w *ChartWriter) writeGroupDirectory(
fs machinery.Filesystem, groupName string,
resources []*unstructured.Unstructured,
) error {
var finalContent bytes.Buffer
Expand Down Expand Up @@ -112,7 +114,8 @@ func (w *ChartWriter) shouldSplitFiles(groupName string) bool {
}

// writeSplitFiles writes each resource in the group to its own file
func (w *ChartWriter) writeSplitFiles(fs machinery.Filesystem, groupName string,
func (w *ChartWriter) writeSplitFiles(
fs machinery.Filesystem, groupName string,
resources []*unstructured.Unstructured,
) error {
// Create the group directory
Expand Down Expand Up @@ -142,7 +145,7 @@ func (w *ChartWriter) generateFileName(resource *unstructured.Unstructured, inde
// Try to use the resource name if available
if name := resource.GetName(); name != "" {
// Remove project prefix from the filename for cleaner file names
projectPrefix := w.templater.projectName + "-"
projectPrefix := w.templater.namePrefix + "-"
fileName := name
if strings.HasPrefix(name, projectPrefix) {
fileName = strings.TrimPrefix(name, projectPrefix)
Expand Down
11 changes: 9 additions & 2 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ const (
// HelmTemplater handles converting YAML content to Helm templates
type HelmTemplater struct {
projectName string
namePrefix string
}

// NewHelmTemplater creates a new Helm templater
func NewHelmTemplater(projectName string) *HelmTemplater {
func NewHelmTemplater(projectName string, namePrefix string) *HelmTemplater {
return &HelmTemplater{
projectName: projectName,
namePrefix: namePrefix,
}
}

Expand Down Expand Up @@ -184,11 +186,16 @@ func (t *HelmTemplater) templateServiceMonitorNames(yamlContent string, resource

// Normalize suffix by stripping the project prefix if present
suffix := origName
prefix := t.projectName + "-"
prefix := t.namePrefix + "-"
if strings.HasPrefix(origName, prefix) {
suffix = strings.TrimPrefix(origName, prefix)
}

// If the prefix is not the project name, we should keep the original name intact
if prefix != t.projectName+"-" {
return yamlContent
}

// Only template if the intended target follows the conventional suffix
// This keeps any custom user-provided names intact
// For default scaffolding, suffix is typically "controller-manager-metrics-monitor"
Expand Down
38 changes: 38 additions & 0 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var _ = Describe("HelmTemplater", func() {
BeforeEach(func() {
templater = &HelmTemplater{
projectName: "test-project",
namePrefix: "test-project",
}
})

Expand Down Expand Up @@ -225,6 +226,43 @@ metadata:
Expect(result).To(ContainSubstring("{{- end }}"))
})

Describe("should generate ServiceMonitor name correctly", func() {
var serviceMonitorResource *unstructured.Unstructured
var content string

BeforeEach(func() {
serviceMonitorResource = &unstructured.Unstructured{}
serviceMonitorResource.SetAPIVersion("monitoring.coreos.com/v1")
serviceMonitorResource.SetKind("ServiceMonitor")
serviceMonitorResource.SetName("test-project-controller-manager-metrics-monitor")

// Test content must end with a newline
// If not, the `name` regex in the ServiceMonitor replacement will never match
content = `apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
name: test-project-controller-manager-metrics-monitor
`
})

It("the namePrefix and project name are the same", func() {
result := templater.ApplyHelmSubstitutions(content, serviceMonitorResource)
// If the namePrefix and default chart name match, should use chart.name template
Expect(result).To(ContainSubstring("{{ include \"chart.name\" . }}"),
"Should use chart.name template when namePrefix and project name match")
})
It("the namePrefix and project name differ", func() {
templaterDiff := &HelmTemplater{
projectName: "project-with-a-long-name",
namePrefix: "test-project",
}
result := templaterDiff.ApplyHelmSubstitutions(content, serviceMonitorResource)
// If the namePrefix and default chart name differ, should use namePrefix variable
Expect(result).ToNot(ContainSubstring("{{ include \"chart.name\" . }}"),
"Should not use chart.name template when namePrefix and project name differ")
})
})

It("should add metrics conditional for metrics services", func() {
serviceResource := &unstructured.Unstructured{}
serviceResource.SetAPIVersion("v1")
Expand Down
24 changes: 24 additions & 0 deletions pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"io"
"os"
"strings"

"gopkg.in/yaml.v3"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -151,3 +152,26 @@ func (p *Parser) categorizeResource(obj *unstructured.Unstructured, resources *P
resources.Other = append(resources.Other, obj)
}
}

func (pr *ParsedResources) EstimatePrefix(projectName string) string {
prefix := projectName
if pr.Deployment != nil {
if name := pr.Deployment.GetName(); name != "" {
deploymentPrefix, found := strings.CutSuffix(projectName, "-controller-manager")
if found {
prefix = deploymentPrefix
}
}
}
// Double check that the prefix is also the prefix for the service names
for _, svc := range pr.Services {
if name := svc.GetName(); name != "" {
if !strings.HasPrefix(name, prefix) {
// If not, fallback to just project name
prefix = projectName
break
}
}
}
return prefix
}
11 changes: 8 additions & 3 deletions ...gins/optional/helm/v2alpha/scaffolds/internal/templates/chart-templates/servicemonitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ var _ machinery.Template = &ServiceMonitor{}
// ServiceMonitor scaffolds a ServiceMonitor for Prometheus monitoring in the Helm chart
type ServiceMonitor struct {
machinery.TemplateMixin
Copy link
Member

Choose a reason for hiding this comment

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

The best solution would be to have the ServiceMonitor from the kustomize install.yaml
Could we look at this option?

Copy link
Author

Choose a reason for hiding this comment

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

When prometheus is enabled in kustomize, it'll use the ServiceMonitor from the kustomize install.yaml - this code path is only called when prometheus is disabled in kustomize.
I assume it's to allow prometheus to be enabled/disabled in the values.yaml of the helm chart.

Copy link
Member

@camilamacedo86 camilamacedo86 Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, that is the reason for we template this one.
But I think we can look for:

name: {{ .NamePrefix }}-controller-manager-metrics-monitor
 
serverName: {{ .NamePrefix }}-controller-manager-metrics-service



Those values in the install.yaml and use here instead of use PROJECT name.

Example:

Copy link
Author

Choose a reason for hiding this comment

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

Ah ok, I think I understand - so take the actual values of the service name from the install.yaml rather than assuming it's the project name or prefix?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the clarification - I'll get something up ASAP for you to take a look at 🙇

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I meant that it's an option that we can consider, rather than an actual option that the user provides! It's not a new flag, and is the only way the data will be passed in to this template!

Copy link
Member

@camilamacedo86 camilamacedo86 Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It will be a new flag for the plugin: fs.StringVar(&p.kustomizePath, "kustomize-path", DefaultKustomizeFile,
That means specific tests for it, and also if we pass one file, we might need to pass others, and it can grow, etc. Also, users will need to pass more than one value by default to properly generate it and bring a bad UX

So, the best approach is we get the values from the install.yaml
We should not consume any other place.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I see - you mean the kustomize path. I'll get something working and run it by you, I have an idea here.

Copy link
Author

Choose a reason for hiding this comment

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

Get data from kustomize output rather than kustomization.yaml changes the way we get the prefix to instead get it from the deployment name. Is this closer to what you were hoping for?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should ONLY:

  1. Check the install.yaml (file-informed).
  2. Look for the controller-manager-metrics-service.
    • Get its full name.
    • Use it to populate the service name.
  3. For the monitor name:
    • Extract all text that comes before.
    • Concatenate it with the default pattern: <prefix obtained from service name>-controller-manager-metrics-monitor

This should make the scaffold populate the correct values automatically, without requiring extra information or flags. Only look for a specific path if one is explicitly provided and do not bring extra burn to keep things maintained.

machinery.ProjectNameMixin

// Prefix
NamePrefix string

// ServiceName is the full name of the metrics service, derived from Kustomize
ServiceName string

// OutputDir specifies the output directory for the chart
OutputDir string
Expand Down Expand Up @@ -59,7 +64,7 @@ metadata:
labels:
{{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
control-plane: controller-manager
name: {{ .ProjectName }}-controller-manager-metrics-monitor
name: {{ .NamePrefix }}-controller-manager-metrics-monitor
namespace: {{ "{{ .Release.Namespace }}" }}
spec:
endpoints:
Expand All @@ -69,7 +74,7 @@ spec:
bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
tlsConfig:
{{ "{{- if .Values.certManager.enable }}" }}
serverName: {{ .ProjectName }}-controller-manager-metrics-service.{{ "{{ .Release.Namespace }}" }}.svc
serverName: {{ .ServiceName }}.{{ "{{ .Release.Namespace }}" }}.svc
# Apply secure TLS configuration with cert-manager
insecureSkipVerify: false
ca:
Expand Down