Skip to content

(helm/v1-alpha) Helm plugin scaffolding is missing control-plane label on metrics service #4591

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.

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

Closed
Jaybee18 opened this issue Mar 5, 2025 · 7 comments · Fixed by #4777
Closed
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@Jaybee18
Copy link

Jaybee18 commented Mar 5, 2025

What broke? What's expected?

Metrics do not work, when generating the helm chart with the kubebuilder helm plugin.

Relevant files:

values.yaml
# [MANAGER]: Manager Deployment Configurations
controllerManager:
  replicas: 1
  container:
    image:
      repository: my-repo.example.com
      tag: v3.2.0
    args:
      - "--leader-elect"
      - "--metrics-bind-address=:8443"
      - "--health-probe-bind-address=:8081"
    resources:
      limits:
        memory: 256Mi
      requests:
        cpu: 10m
        memory: 64Mi
    livenessProbe:
      initialDelaySeconds: 15
      periodSeconds: 20
      httpGet:
        path: /healthz
        port: 8081
    readinessProbe:
      initialDelaySeconds: 5
      periodSeconds: 10
      httpGet:
        path: /readyz
        port: 8081
    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        drop:
          - "ALL"
  imagePullSecrets:
    - name: my-pull-secret
  securityContext:
    runAsNonRoot: true
    seccompProfile:
      type: RuntimeDefault
  terminationGracePeriodSeconds: 10
  serviceAccountName: ddp-operator-controller-manager

# [RBAC]: To enable RBAC (Permissions) configurations
rbac:
  enable: true

# [CRDs]: To enable the CRDs
crd:
  # This option determines whether the CRDs are included
  # in the installation process.
  enable: false

  # Enabling this option adds the "helm.sh/resource-policy": keep
  # annotation to the CRD, ensuring it remains installed even when
  # the Helm release is uninstalled.
  # NOTE: Removing the CRDs will also remove all cert-manager CR(s)
  # (Certificates, Issuers, ...) due to garbage collection.
  keep: true

# [METRICS]: Set to true to generate manifests for exporting metrics.
# To disable metrics export set false, and ensure that the
# ControllerManager argument "--metrics-bind-address=:8443" is removed.
metrics:
  enable: true

# [PROMETHEUS]: To enable a ServiceMonitor to export metrics to Prometheus set true
prometheus:
  enable: true

# [CERT-MANAGER]: To enable cert-manager injection to webhooks set true
certmanager:
  enable: false

# [NETWORK POLICIES]: To enable NetworkPolicies set true
networkPolicy:
  enable: false
cmd/main.go
/*
Copyright 2025.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
	"crypto/tls"
	"flag"
	"os"
	"path/filepath"

	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
	// to ensure that exec-entrypoint and run can make use of them.
	_ "k8s.io/client-go/plugin/pkg/client/auth"

	"k8s.io/apimachinery/pkg/runtime"
	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
	ctrl "sigs.k8s.io/controller-runtime"
	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
	"sigs.k8s.io/controller-runtime/pkg/healthz"
	"sigs.k8s.io/controller-runtime/pkg/log/zap"
	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
	"sigs.k8s.io/controller-runtime/pkg/webhook"

	/* ... */

	// +kubebuilder:scaffold:imports
)

var (
	scheme   = runtime.NewScheme()
	setupLog = ctrl.Log.WithName("setup")
)

func init() {
	utilruntime.Must(clientgoscheme.AddToScheme(scheme))

	/* ... */
	// +kubebuilder:scaffold:scheme
}

// nolint:gocyclo
func main() {
	var metricsAddr string
	var metricsCertPath, metricsCertName, metricsCertKey string
	var webhookCertPath, webhookCertName, webhookCertKey string
	var enableLeaderElection bool
	var probeAddr string
	var secureMetrics bool
	var enableHTTP2 bool
	var tlsOpts []func(*tls.Config)
	flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
		"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
		"Enable leader election for controller manager. "+
			"Enabling this will ensure there is only one active controller manager.")
	flag.BoolVar(&secureMetrics, "metrics-secure", true,
		"If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.")
	flag.StringVar(&webhookCertPath, "webhook-cert-path", "", "The directory that contains the webhook certificate.")
	flag.StringVar(&webhookCertName, "webhook-cert-name", "tls.crt", "The name of the webhook certificate file.")
	flag.StringVar(&webhookCertKey, "webhook-cert-key", "tls.key", "The name of the webhook key file.")
	flag.StringVar(&metricsCertPath, "metrics-cert-path", "",
		"The directory that contains the metrics server certificate.")
	flag.StringVar(&metricsCertName, "metrics-cert-name", "tls.crt", "The name of the metrics server certificate file.")
	flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.")
	flag.BoolVar(&enableHTTP2, "enable-http2", false,
		"If set, HTTP/2 will be enabled for the metrics and webhook servers")
	opts := zap.Options{
		Development: true,
	}
	opts.BindFlags(flag.CommandLine)
	flag.Parse()

	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

	// if the enable-http2 flag is false (the default), http/2 should be disabled
	// due to its vulnerabilities. More specifically, disabling http/2 will
	// prevent from being vulnerable to the HTTP/2 Stream Cancellation and
	// Rapid Reset CVEs. For more information see:
	// - https://github.com/advisories/GHSA-qppj-fm5r-hxr3
	// - https://github.com/advisories/GHSA-4374-p667-p6c8
	disableHTTP2 := func(c *tls.Config) {
		setupLog.Info("disabling http/2")
		c.NextProtos = []string{"http/1.1"}
	}

	if !enableHTTP2 {
		tlsOpts = append(tlsOpts, disableHTTP2)
	}

	// Create watchers for metrics and webhooks certificates
	var metricsCertWatcher, webhookCertWatcher *certwatcher.CertWatcher

	// Initial webhook TLS options
	webhookTLSOpts := tlsOpts

	if len(webhookCertPath) > 0 {
		setupLog.Info("Initializing webhook certificate watcher using provided certificates",
			"webhook-cert-path", webhookCertPath, "webhook-cert-name", webhookCertName, "webhook-cert-key", webhookCertKey)

		var err error
		webhookCertWatcher, err = certwatcher.New(
			filepath.Join(webhookCertPath, webhookCertName),
			filepath.Join(webhookCertPath, webhookCertKey),
		)
		if err != nil {
			setupLog.Error(err, "Failed to initialize webhook certificate watcher")
			os.Exit(1)
		}

		webhookTLSOpts = append(webhookTLSOpts, func(config *tls.Config) {
			config.GetCertificate = webhookCertWatcher.GetCertificate
		})
	}

	webhookServer := webhook.NewServer(webhook.Options{
		TLSOpts: webhookTLSOpts,
	})

	// Metrics endpoint is enabled in 'config/default/kustomization.yaml'. The Metrics options configure the server.
	// More info:
	// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/server
	// - https://book.kubebuilder.io/reference/metrics.html
	metricsServerOptions := metricsserver.Options{
		BindAddress:   metricsAddr,
		SecureServing: secureMetrics,
		TLSOpts:       tlsOpts,
	}

	if secureMetrics {
		// FilterProvider is used to protect the metrics endpoint with authn/authz.
		// These configurations ensure that only authorized users and service accounts
		// can access the metrics endpoint. The RBAC are configured in 'config/rbac/kustomization.yaml'. More info:
		// https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/metrics/filters#WithAuthenticationAndAuthorization
		metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization
	}

	// If the certificate is not specified, controller-runtime will automatically
	// generate self-signed certificates for the metrics server. While convenient for development and testing,
	// this setup is not recommended for production.
	//
	// TODO(user): If you enable certManager, uncomment the following lines:
	// - [METRICS-WITH-CERTS] at config/default/kustomization.yaml to generate and use certificates
	// managed by cert-manager for the metrics server.
	// - [PROMETHEUS-WITH-CERTS] at config/prometheus/kustomization.yaml for TLS certification.
	if len(metricsCertPath) > 0 {
		setupLog.Info("Initializing metrics certificate watcher using provided certificates",
			"metrics-cert-path", metricsCertPath, "metrics-cert-name", metricsCertName, "metrics-cert-key", metricsCertKey)

		var err error
		metricsCertWatcher, err = certwatcher.New(
			filepath.Join(metricsCertPath, metricsCertName),
			filepath.Join(metricsCertPath, metricsCertKey),
		)
		if err != nil {
			setupLog.Error(err, "to initialize metrics certificate watcher", "error", err)
			os.Exit(1)
		}

		metricsServerOptions.TLSOpts = append(metricsServerOptions.TLSOpts, func(config *tls.Config) {
			config.GetCertificate = metricsCertWatcher.GetCertificate
		})
	}

	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
		Scheme:                 scheme,
		Metrics:                metricsServerOptions,
		WebhookServer:          webhookServer,
		HealthProbeBindAddress: probeAddr,
		LeaderElection:         enableLeaderElection,
		LeaderElectionID:       "32cd121a.ddp.d-velop.de",
		// LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily
		// when the Manager ends. This requires the binary to immediately end when the
		// Manager is stopped, otherwise, this setting is unsafe. Setting this significantly
		// speeds up voluntary leader transitions as the new leader don't have to wait
		// LeaseDuration time first.
		//
		// In the default scaffold provided, the program ends immediately after
		// the manager stops, so would be fine to enable this option. However,
		// if you are doing or is intended to do any operation such as perform cleanups
		// after the manager stops then its usage might be unsafe.
		// LeaderElectionReleaseOnCancel: true,
	})
	if err != nil {
		setupLog.Error(err, "unable to start manager")
		os.Exit(1)
	}

	if err = (&compositioncontroller.S3StorageReconciler{
		Client:   mgr.GetClient(),
		Scheme:   mgr.GetScheme(),
		Recorder: mgr.GetEventRecorderFor("S3StorageReconciler"),
	}).SetupWithManager(mgr); err != nil {
		setupLog.Error(err, "unable to create controller", "controller", "S3Storage")
		os.Exit(1)
	}

        /* ... */

	if err = (&miniocontroller.PolicyReconciler{
		Client:   mgr.GetClient(),
		Scheme:   mgr.GetScheme(),
		Recorder: mgr.GetEventRecorderFor("PolicyReconciler"),
	}).SetupWithManager(mgr); err != nil {
		setupLog.Error(err, "unable to create controller", "controller", "Policy")
		os.Exit(1)
	}
	// +kubebuilder:scaffold:builder

	if metricsCertWatcher != nil {
		setupLog.Info("Adding metrics certificate watcher to manager")
		if err := mgr.Add(metricsCertWatcher); err != nil {
			setupLog.Error(err, "unable to add metrics certificate watcher to manager")
			os.Exit(1)
		}
	}

	if webhookCertWatcher != nil {
		setupLog.Info("Adding webhook certificate watcher to manager")
		if err := mgr.Add(webhookCertWatcher); err != nil {
			setupLog.Error(err, "unable to add webhook certificate watcher to manager")
			os.Exit(1)
		}
	}

	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
		setupLog.Error(err, "unable to set up health check")
		os.Exit(1)
	}
	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
		setupLog.Error(err, "unable to set up ready check")
		os.Exit(1)
	}

	setupLog.Info("starting manager")
	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
		setupLog.Error(err, "problem running manager")
		os.Exit(1)
	}
}

Further details:
When not using the helm plugin, i.e. not generating the helm chart and just installing via the resources in the config/ folder, the metrics work just fine. Though when generating the helm chart, and then installing the operator that way, the metrics do not show up in our prometheus.

I believe this is because the helm plugin forgets the control-plane: controller-manager label on the metrics Service in the scaffolding here between lines 51 and 52:

/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package prometheus

import (
	"path/filepath"

	"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
)

var _ machinery.Template = &Monitor{}

// Monitor scaffolds the ServiceMonitor for Prometheus in the Helm chart
type Monitor struct {
	machinery.TemplateMixin
	machinery.ProjectNameMixin
}

// SetTemplateDefaults sets the default template configuration
func (f *Monitor) SetTemplateDefaults() error {
	if f.Path == "" {
		f.Path = filepath.Join("dist", "chart", "templates", "prometheus", "monitor.yaml")
	}

	f.TemplateBody = monitorTemplate

	f.IfExistsAction = machinery.OverwriteFile

	return nil
}

const monitorTemplate = `# To integrate with Prometheus.
{{ "{{- if .Values.prometheus.enable }}" }}
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  labels:
    {{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
  name: {{ .ProjectName }}-controller-manager-metrics-monitor
  namespace: {{ "{{ .Release.Namespace }}" }}
spec:
  endpoints:
    - path: /metrics
      port: https
      scheme: https
      bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
      tlsConfig:
        {{ "{{- if .Values.certmanager.enable }}" }}
        serverName: {{ .ProjectName }}-controller-manager-metrics-service.{{ "{{ .Release.Namespace }}" }}.svc
        # Apply secure TLS configuration with cert-manager
        insecureSkipVerify: false
        ca:
          secret:
            name: metrics-server-cert
            key: ca.crt
        cert:
          secret:
            name: metrics-server-cert
            key: tls.crt
        keySecret:
          name: metrics-server-cert
          key: tls.key
        {{ "{{- else }}" }}
        # Development/Test mode (insecure configuration)
        insecureSkipVerify: true
        {{ "{{- end }}" }}
  selector:
    matchLabels:
      control-plane: controller-manager
{{ "{{- end }}" }}
`

Because the Service does not have this label, the ServiceMonitor cannot find it and therefore cannot provide the metrics.
I believe the label is missing in that line (as opposed to the chart.labels list from the helm template), because the controller manager deployment also has the label in that position (line 71):

manager.go
/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package manager

import (
	"path/filepath"

	"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
)

var _ machinery.Template = &Deployment{}

// Deployment scaffolds the manager Deployment for the Helm chart
type Deployment struct {
	machinery.TemplateMixin
	machinery.ProjectNameMixin

	// DeployImages if true will scaffold the env with the images
	DeployImages bool
	// Force if true allow overwrite the scaffolded file
	Force bool
	// HasWebhooks is true when webhooks were found in the config
	HasWebhooks bool
}

// SetTemplateDefaults sets the default template configuration
func (f *Deployment) SetTemplateDefaults() error {
	if f.Path == "" {
		f.Path = filepath.Join("dist", "chart", "templates", "manager", "manager.yaml")
	}

	f.TemplateBody = managerDeploymentTemplate

	if f.Force {
		f.IfExistsAction = machinery.OverwriteFile
	} else {
		f.IfExistsAction = machinery.SkipFile
	}

	return nil
}

//nolint:lll
const managerDeploymentTemplate = `apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ .ProjectName }}-controller-manager
  namespace: {{ "{{ .Release.Namespace }}" }}
  labels:
    {{ "{{- include \"chart.labels\" . | nindent 4 }}" }}
    control-plane: controller-manager
spec:
  replicas:  {{ "{{ .Values.controllerManager.replicas }}" }}
  selector:
    matchLabels:
      {{ "{{- include \"chart.selectorLabels\" . | nindent 6 }}" }}
      control-plane: controller-manager
  template:
    metadata:
      annotations:
        kubectl.kubernetes.io/default-container: manager
      labels:
        {{ "{{- include \"chart.labels\" . | nindent 8 }}" }}
        control-plane: controller-manager
        {{ "{{- if and .Values.controllerManager.pod .Values.controllerManager.pod.labels }}" }}
        {{ "{{- range $key, $value := .Values.controllerManager.pod.labels }}" }}
        {{ "{{ $key }}" }}: {{ "{{ $value }}" }}
        {{ "{{- end }}" }}
        {{ "{{- end }}" }}
    spec:
      containers:
        - name: manager
          args:
            {{ "{{- range .Values.controllerManager.container.args }}" }}
            - {{ "{{ . }}" }}
            {{ "{{- end }}" }}
          command:
            - /manager
          image: {{ "{{ .Values.controllerManager.container.image.repository }}" }}:{{ "{{ .Values.controllerManager.container.image.tag }}" }}
          {{ "{{- if .Values.controllerManager.container.env }}" }}
          env:
            {{ "{{- range $key, $value := .Values.controllerManager.container.env }}" }}
            - name: {{ "{{ $key }}" }}
              value: {{ "{{ $value }}" }}
            {{ "{{- end }}" }}
          {{ "{{- end }}" }}
          livenessProbe:
            {{ "{{- toYaml .Values.controllerManager.container.livenessProbe | nindent 12 }}" }}
          readinessProbe:
            {{ "{{- toYaml .Values.controllerManager.container.readinessProbe | nindent 12 }}" }}
{{- if .HasWebhooks }}
          {{ "{{- if .Values.webhook.enable }}" }}
          ports:
            - containerPort: 9443
              name: webhook-server
              protocol: TCP
          {{ "{{- end }}" }}
{{- end }}
          resources:
            {{ "{{- toYaml .Values.controllerManager.container.resources | nindent 12 }}" }}
          securityContext:
            {{ "{{- toYaml .Values.controllerManager.container.securityContext | nindent 12 }}" }}
          {{ "{{- if and .Values.certmanager.enable (or .Values.webhook.enable .Values.metrics.enable) }}" }}
          volumeMounts:
{{- if .HasWebhooks }}
            {{ "{{- if and .Values.webhook.enable .Values.certmanager.enable }}" }}
            - name: webhook-cert
              mountPath: /tmp/k8s-webhook-server/serving-certs
              readOnly: true
            {{ "{{- end }}" }}
{{- end }}
            {{ "{{- if and .Values.metrics.enable .Values.certmanager.enable }}" }}
            - name: metrics-certs
              mountPath: /tmp/k8s-metrics-server/metrics-certs
              readOnly: true
            {{ "{{- end }}" }}
          {{ "{{- end }}" }}
      securityContext:
        {{ "{{- toYaml .Values.controllerManager.securityContext | nindent 8 }}" }}
      serviceAccountName: {{ "{{ .Values.controllerManager.serviceAccountName }}" }}
      terminationGracePeriodSeconds: {{ "{{ .Values.controllerManager.terminationGracePeriodSeconds }}" }}
      {{ "{{- if and .Values.certmanager.enable (or .Values.webhook.enable .Values.metrics.enable) }}" }}
      volumes:
{{- if .HasWebhooks }}
        {{ "{{- if and .Values.webhook.enable .Values.certmanager.enable }}" }}
        - name: webhook-cert
          secret:
            secretName: webhook-server-cert
        {{ "{{- end }}" }}
{{- end }}
        {{ "{{- if and .Values.metrics.enable .Values.certmanager.enable }}" }}
        - name: metrics-certs
          secret:
            secretName: metrics-server-cert
        {{ "{{- end }}" }}
      {{ "{{- end }}" }}
`

Reproducing this issue

No response

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"4.5.0", KubernetesVendor:"unknown", GitCommit:"7153119ca900994b70507edbde59771ac824f2d9", BuildDate:"2025-01-21T08:28:36Z", GoOs:"darwin", GoArch:"arm64"}

PROJECT version

3

Plugin versions

go.kubebuilder.io/v4

Other versions

Using helm/v1-alpha plugin to generate the helm chart

Extra Labels

No response

@Jaybee18 Jaybee18 added the kind/bug Categorizes issue or PR as related to a bug. label Mar 5, 2025
@Jaybee18 Jaybee18 changed the title Helm plugin scaffolding is missing control-plane label on metrics service (helm/v1-alpha) Helm plugin scaffolding is missing control-plane label on metrics service Mar 5, 2025
@Jaybee18
Copy link
Author

Jaybee18 commented Mar 5, 2025

The ServiceMonitor is also missing that label, but in this case it has no impact on functionality

@mkarlheim
Copy link

I just noticed the exact same issue.

The ServiceMonitor uses control-plane: controller-manager as label selector, but the Service does not have the label. This leads to the loss of metrics.

In Kustomize the label control-plane: controller-manager was added to the Service and ServiceMonitor, which seems to be missing in the generated helm chart.

@sarthaksarthak9
Copy link
Member

/assign

@Kuzuri247
Copy link
Contributor

This issue is resolved @camilamacedo86

@camilamacedo86
Copy link
Member

Hi @Kuzuri247

We need to merge the PR to do so, I need to ensure that the changes made sort out this issue.
Would you like to help us to review this one?

@Kuzuri247
Copy link
Contributor

Yes I can help @camilamacedo86

@Jaybee18
Copy link
Author

@sarthaksarthak9 @Kuzuri247 @camilamacedo86 Thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
5 participants