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 5 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
7 changes: 6 additions & 1 deletion pkg/plugins/optional/helm/v2alpha/edit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const (
DefaultManifestsFile = "dist/install.yaml"
// DefaultOutputDir is the default output directory for Helm charts
DefaultOutputDir = "dist"
// DefaultKustomizeFile is the default path for kustomization.yaml
DefaultKustomizeFile = "config/default/kustomization.yaml"
)

var _ plugin.EditSubcommand = &editSubcommand{}
Expand All @@ -46,6 +48,7 @@ type editSubcommand struct {
force bool
manifestsFile string
outputDir string
kustomizePath string
}

//nolint:lll
Expand Down Expand Up @@ -98,6 +101,8 @@ func (p *editSubcommand) BindFlags(fs *pflag.FlagSet) {
fs.BoolVar(&p.force, "force", false, "if true, regenerates all the files")
fs.StringVar(&p.manifestsFile, "manifests", DefaultManifestsFile,
"path to the YAML file containing Kubernetes manifests from kustomize output")
fs.StringVar(&p.kustomizePath, "kustomize-path", DefaultKustomizeFile,
Copy link
Member

Choose a reason for hiding this comment

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

I do not think that we should pass the path here, otherwise we will need a flag for each file.
In this case, we might want to get the config from PROJECT file which has the project name instead of check the kustomize file wdyt?

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately, there's no reference to the entrypoint for Kustomize in the PROJECT file, and there's no way to infer the prefix someone uses in Kustomize outside of the kustomization.yaml itself. Kubebuilder conventions put it in config/default/kustomization.yaml. Originally, I left this as unconfigurable, and simply assumed that the kustomization.yaml would always be in this path.

The best we have to get this programatically is the Makefile, which uses it in the build-installer target:

	$(KUSTOMIZE) build config/default > dist/install.yaml

The config/default path is what we'd use, and we'd simply look for a kustomization.yaml in there. I'm not sure if that's any better than simply allowing the desired kustomization file to be passed through though.

Happy to defer to your expertise here!

"path to the kustomization.yaml file used to generate the manifests file")
fs.StringVar(&p.outputDir, "output-dir", DefaultOutputDir, "output directory for the generated Helm chart")
}

Expand All @@ -114,7 +119,7 @@ func (p *editSubcommand) Scaffold(fs machinery.Filesystem) error {
}
}

scaffolder := scaffolds.NewKustomizeHelmScaffolder(p.config, p.force, p.manifestsFile, p.outputDir)
scaffolder := scaffolds.NewKustomizeHelmScaffolder(p.config, p.force, p.manifestsFile, p.outputDir, p.kustomizePath)
scaffolder.InjectFS(fs)
err := scaffolder.Scaffold()
if err != nil {
Expand Down
24 changes: 20 additions & 4 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 @@ -45,15 +45,19 @@ type editKustomizeScaffolder struct {
force bool
manifestsFile string
outputDir string
kustomizeFile string
}

// NewKustomizeHelmScaffolder returns a new Scaffolder for HelmPlugin using kustomize output
func NewKustomizeHelmScaffolder(cfg config.Config, force bool, manifestsFile, outputDir string) plugins.Scaffolder {
func NewKustomizeHelmScaffolder(
cfg config.Config, force bool, manifestsFile, outputDir, kustomizeFile string,
) plugins.Scaffolder {
return &editKustomizeScaffolder{
config: cfg,
force: force,
manifestsFile: manifestsFile,
outputDir: outputDir,
kustomizeFile: kustomizeFile,
}
}

Expand Down Expand Up @@ -109,7 +113,18 @@ 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)

// Read namePrefix from kustomization.yaml
// falls back to project name if not found/configured
namePrefix := kustomize.ReadNamePrefix(s.kustomizeFile)
if namePrefix == "" {
namePrefix = s.config.GetProjectName()
} else {
// Remove trailing dash if present
namePrefix = strings.TrimSuffix(namePrefix, "-")
}

chartConverter := kustomize.NewChartConverter(resources, s.config.GetProjectName(), namePrefix, s.outputDir)
deploymentConfig := chartConverter.ExtractDeploymentConfig()

// Create scaffold for standard Helm chart files
Expand All @@ -119,7 +134,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 +150,7 @@ 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})
chartFiles = append(chartFiles, &charttemplates.ServiceMonitor{OutputDir: s.outputDir, NamePrefix: namePrefix})
}

// 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
30 changes: 30 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 @@
BeforeEach(func() {
templater = &HelmTemplater{
projectName: "test-project",
namePrefix: "test-project",
}
})

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

Describe("should generate ServiceMonitor name correctly", func() {
serviceMonitorResource := &unstructured.Unstructured{}

Check failure on line 230 in pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

use BeforeEach() to assign variable serviceMonitorResource (ginkgolinter)
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

Check failure on line 237 in pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

use BeforeEach() to assign variable content (ginkgolinter)
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")

Check failure on line 245 in pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

The line is 146 characters long, which exceeds the maximum of 120 characters. (lll)
})
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")

Check failure on line 254 in pkg/plugins/optional/helm/v2alpha/scaffolds/internal/kustomize/helm_templater_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

The line is 154 characters long, which exceeds the maximum of 120 characters. (lll)
})
})

It("should add metrics conditional for metrics services", func() {
serviceResource := &unstructured.Unstructured{}
serviceResource.SetAPIVersion("v1")
Expand Down
19 changes: 19 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 @@ -151,3 +151,22 @@ func (p *Parser) categorizeResource(obj *unstructured.Unstructured, resources *P
resources.Other = append(resources.Other, obj)
}
}

// ReadNamePrefix reads the namePrefix from a kustomization.yaml file
// Returns empty string if the file doesn't exist or has no namePrefix
func ReadNamePrefix(kustomizationPath string) string {
data, err := os.ReadFile(kustomizationPath)
if err != nil {
return ""
}

var kustomization struct {
NamePrefix string `yaml:"namePrefix"`
}

if err := yaml.Unmarshal(data, &kustomization); err != nil {
return ""
}

return kustomization.NamePrefix
}
7 changes: 5 additions & 2 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 @@ -31,6 +31,9 @@ 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
Member

Choose a reason for hiding this comment

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

But then we need to remove the rest only get the data from the install.yaml
We should avoid adding flags since it will make it harder to keep things maintained

Could we keep the solution here only looking for the install.yaml to get this value?

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?

machinery.ProjectNameMixin

// Prefix
NamePrefix string

// OutputDir specifies the output directory for the chart
OutputDir string
}
Expand Down Expand Up @@ -59,7 +62,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 +72,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: {{ .NamePrefix }}-controller-manager-metrics-service.{{ "{{ .Release.Namespace }}" }}.svc
# Apply secure TLS configuration with cert-manager
insecureSkipVerify: false
ca:
Expand Down
Loading