From 029453e55d87016b8da87e3e41904b240b4ba4fb Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Mon, 20 Jul 2026 21:36:36 +0000 Subject: [PATCH 01/16] refactor: extract spec logic and builders --- pkg/migrate/helpers.go | 68 +++++++++++++++ pkg/migrate/podmonitor.go | 174 +++++++++++--------------------------- pkg/migrate/types.go | 12 +++ 3 files changed, 131 insertions(+), 123 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 8e56f2f936..ad0e31c16e 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -28,6 +28,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation" ) @@ -963,3 +964,70 @@ func convertLimits(sampleLimit, labelLimit, labelNameLengthLimit, labelValueLeng } return limits } + +// buildPodMonitoring constructs a GMP PodMonitoring resource from common spec. +func buildPodMonitoring( + srcMeta metav1.ObjectMeta, + targetNamespace string, + spec *commonMonitorSpec, + logger *slog.Logger, +) (*unstructured.Unstructured, error) { + gmpPM := &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: CopyObjectMeta(srcMeta, targetNamespace, logger), + Spec: monitoringv1.PodMonitoringSpec{ + Selector: spec.mergedSelector, + Endpoints: spec.endpoints, + TargetLabels: monitoringv1.TargetLabels{ + FromPod: spec.mergedFromPod, + Metadata: spec.metadata, + }, + Limits: spec.limits, + FilterRunning: spec.filterRunning, + }, + } + + unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpPM) + if err != nil { + return nil, fmt.Errorf("failed to marshal PodMonitoring: %w", err) + } + + u := &unstructured.Unstructured{Object: unstructuredMap} + u.SetAPIVersion(GMPAPIVersion) + u.SetKind(KindPodMonitoring) + + return u, nil +} + +// buildClusterPodMonitoring constructs a GMP ClusterPodMonitoring resource from common spec. +func buildClusterPodMonitoring( + srcMeta metav1.ObjectMeta, + spec *commonMonitorSpec, + logger *slog.Logger, +) (*unstructured.Unstructured, error) { + gmpCPM := &monitoringv1.ClusterPodMonitoring{ + TypeMeta: BuildTypeMeta(KindClusterPodMonitoring), + ObjectMeta: CopyObjectMeta(srcMeta, "", logger), + Spec: monitoringv1.ClusterPodMonitoringSpec{ + Selector: spec.mergedSelector, + Endpoints: spec.endpoints, + TargetLabels: monitoringv1.ClusterTargetLabels{ + FromPod: spec.mergedFromPod, + Metadata: spec.metadata, + }, + Limits: spec.limits, + FilterRunning: spec.filterRunning, + }, + } + + unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpCPM) + if err != nil { + return nil, fmt.Errorf("failed to marshal ClusterPodMonitoring: %w", err) + } + + u := &unstructured.Unstructured{Object: unstructuredMap} + u.SetAPIVersion(GMPAPIVersion) + u.SetKind(KindClusterPodMonitoring) + + return u, nil +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index e6ff6a6bbd..8ae9a671de 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -271,7 +271,7 @@ func (c *PodMonitorConverter) convertEndpoints( return gmpEndpoints, nil } -func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { +func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache, isCluster bool) (*commonMonitorSpec, error) { convCtx := &conversionContext{ logger: logger, cache: cache, @@ -279,17 +279,17 @@ func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonit } rules, err := extractPreScrapeRelabelings(logger, pm.Spec.PodMetricsEndpoints) if err != nil { - return nil, nil, err + return nil, err } endpoints, err := c.convertEndpoints(convCtx, pm.Spec.PodMetricsEndpoints, rules.PerEndpoint) if err != nil { - return nil, nil, err + return nil, err } mergedFromPod := mergeFromPod(logger, convertTargetLabels(logger, pm.Spec.PodTargetLabels, pm.Spec.JobLabel, "Pod"), rules.ResourceCombined.FromPod) mergedSelector, err := mergeLabelSelector(pm.Spec.Selector, rules.ResourceCombined.MatchLabels, rules.ResourceCombined.MatchExpressions) if err != nil { - return nil, nil, err + return nil, err } if len(mergedSelector.MatchLabels) == 0 && len(mergedSelector.MatchExpressions) == 0 { logger.Warn("Resulting PodMonitoring selector is empty. It will select and scrape all pods in this namespace. Verify if this is intended.") @@ -307,18 +307,34 @@ func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonit } var filteredMetadata *[]string - if rules.ResourceCombined.Metadata != nil { - union := unionMetadata(*rules.ResourceCombined.Metadata, namespacedMetadataDefaults) - var md []string - for _, m := range union { - if m != export.KeyNamespace { - md = append(md, m) + if isCluster { + if rules.ResourceCombined.Metadata != nil { + union := unionMetadata(*rules.ResourceCombined.Metadata, clusterMetadataDefaults) + filteredMetadata = &union + } + if pm.Spec.AttachMetadata != nil && pm.Spec.AttachMetadata.Node != nil && *pm.Spec.AttachMetadata.Node { + if filteredMetadata == nil { + union := unionMetadata([]string{labelNode}, clusterMetadataDefaults) + filteredMetadata = &union } else { - logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The metadata entry has been omitted .") + union := unionMetadata([]string{labelNode}, *filteredMetadata) + filteredMetadata = &union } } - if len(md) > 0 { - filteredMetadata = &md + } else { + if rules.ResourceCombined.Metadata != nil { + union := unionMetadata(*rules.ResourceCombined.Metadata, namespacedMetadataDefaults) + var md []string + for _, m := range union { + if m != export.KeyNamespace { + md = append(md, m) + } else { + logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The metadata entry has been omitted .") + } + } + if len(md) > 0 { + filteredMetadata = &md + } } } @@ -348,137 +364,49 @@ func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonit falseVal := false filterRunning = &falseVal if hasTrue { - logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally on the PodMonitoring resource.") + logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally.") } } limits := convertLimits(pm.Spec.SampleLimit, pm.Spec.LabelLimit, pm.Spec.LabelNameLengthLimit, pm.Spec.LabelValueLengthLimit) - gmpPM := &monitoringv1.PodMonitoring{ - TypeMeta: BuildTypeMeta(KindPodMonitoring), - ObjectMeta: CopyObjectMeta(pm.ObjectMeta, pm.Namespace, logger), - Spec: monitoringv1.PodMonitoringSpec{ - Selector: mergedSelector, - Endpoints: endpoints, - TargetLabels: monitoringv1.TargetLabels{ - FromPod: mergedFromPod, - Metadata: filteredMetadata, - }, - Limits: limits, - FilterRunning: filterRunning, - }, - } - - unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpPM) - if err != nil { - return nil, nil, fmt.Errorf("failed to marshal PodMonitoring: %w", err) - } - - u := &unstructured.Unstructured{Object: unstructuredMap} - u.SetAPIVersion(GMPAPIVersion) - u.SetKind(KindPodMonitoring) - - return u, convCtx.getGeneratedSecrets(), nil + return &commonMonitorSpec{ + endpoints: endpoints, + mergedFromPod: mergedFromPod, + mergedSelector: mergedSelector, + metadata: filteredMetadata, + filterRunning: filterRunning, + limits: limits, + generatedSecrets: convCtx.getGeneratedSecrets(), + }, nil } -func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - convCtx := &conversionContext{ - logger: logger, - cache: cache, - namespace: pm.Namespace, - } - rules, err := extractPreScrapeRelabelings(logger, pm.Spec.PodMetricsEndpoints) - if err != nil { - return nil, nil, err - } - endpoints, err := c.convertEndpoints(convCtx, pm.Spec.PodMetricsEndpoints, rules.PerEndpoint) +func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + res, err := c.convertMonitorSpec(pm, logger, cache, false) if err != nil { return nil, nil, err } - mergedFromPod := mergeFromPod(logger, convertTargetLabels(logger, pm.Spec.PodTargetLabels, pm.Spec.JobLabel, "Pod"), rules.ResourceCombined.FromPod) - mergedSelector, err := mergeLabelSelector(pm.Spec.Selector, rules.ResourceCombined.MatchLabels, rules.ResourceCombined.MatchExpressions) + u, err := buildPodMonitoring(pm.ObjectMeta, pm.Namespace, res, logger) if err != nil { return nil, nil, err } - if len(mergedSelector.MatchLabels) == 0 && len(mergedSelector.MatchExpressions) == 0 { - logger.Warn("Resulting ClusterPodMonitoring selector is empty. It will select and scrape all pods across all namespaces. Verify if this is intended.") - } - // Spec-level warnings for unsupported fields. - warnUnsupportedSpecFields(logger, &pm.Spec) - // TODO(M2): Resolve and merge ScrapeClass configurations from Prometheus CR if scrapeClassName is specified. - if pm.Spec.ScrapeClassName != nil && *pm.Spec.ScrapeClassName != "" { - logger.Warn(fmt.Sprintf("ScrapeClass %q was not found in the inputs. The 'scrapeClassName' field has been dropped and inherited settings will be lost.", *pm.Spec.ScrapeClassName)) - } - // Check against the PrometheusProto enum value. - if slices.Contains(pm.Spec.ScrapeProtocols, scrapeProtocolPrometheusProto) { - logger.Warn("Scrape protocol settings (scrapeProtocols) requiring Protobuf are unsupported. Scrapes may fail if target lacks text fallback.") - } - - var filteredMetadata *[]string - if rules.ResourceCombined.Metadata != nil { - union := unionMetadata(*rules.ResourceCombined.Metadata, clusterMetadataDefaults) - filteredMetadata = &union - } - - // In GMP, Metadata: nil on a ClusterPodMonitoring defaults to emitting namespace and other cluster defaults. - // When setting Metadata explicitly for AttachMetadata.Node, we must merge clusterMetadataDefaults so that namespace is not dropped. - if pm.Spec.AttachMetadata != nil && pm.Spec.AttachMetadata.Node != nil && *pm.Spec.AttachMetadata.Node { - if filteredMetadata == nil { - union := unionMetadata([]string{labelNode}, clusterMetadataDefaults) - filteredMetadata = &union - } else { - union := unionMetadata([]string{labelNode}, *filteredMetadata) - filteredMetadata = &union - } - } - - var hasFalse, hasTrue bool - for _, ep := range pm.Spec.PodMetricsEndpoints { - if ep.FilterRunning != nil && !*ep.FilterRunning { - hasFalse = true - } else { - hasTrue = true - } - } - // A nil filterRunning defaults to true in the GMP operator. - var filterRunning *bool - if hasFalse { - falseVal := false - filterRunning = &falseVal - if hasTrue { - logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally on the ClusterPodMonitoring resource.") - } - } - - limits := convertLimits(pm.Spec.SampleLimit, pm.Spec.LabelLimit, pm.Spec.LabelNameLengthLimit, pm.Spec.LabelValueLengthLimit) + return u, res.generatedSecrets, nil +} - gmpCPM := &monitoringv1.ClusterPodMonitoring{ - TypeMeta: BuildTypeMeta(KindClusterPodMonitoring), - ObjectMeta: CopyObjectMeta(pm.ObjectMeta, "", logger), - Spec: monitoringv1.ClusterPodMonitoringSpec{ - Selector: mergedSelector, - Endpoints: endpoints, - TargetLabels: monitoringv1.ClusterTargetLabels{ - FromPod: mergedFromPod, - Metadata: filteredMetadata, - }, - Limits: limits, - FilterRunning: filterRunning, - }, +func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + res, err := c.convertMonitorSpec(pm, logger, cache, true) + if err != nil { + return nil, nil, err } - unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpCPM) + u, err := buildClusterPodMonitoring(pm.ObjectMeta, res, logger) if err != nil { - return nil, nil, fmt.Errorf("failed to marshal ClusterPodMonitoring: %w", err) + return nil, nil, err } - u := &unstructured.Unstructured{Object: unstructuredMap} - u.SetAPIVersion(GMPAPIVersion) - u.SetKind(KindClusterPodMonitoring) - - return u, convCtx.getGeneratedSecrets(), nil + return u, res.generatedSecrets, nil } func unionMetadata(extracted []string, defaults []string) []string { diff --git a/pkg/migrate/types.go b/pkg/migrate/types.go index 8fab955448..aff2bd7f56 100644 --- a/pkg/migrate/types.go +++ b/pkg/migrate/types.go @@ -20,6 +20,8 @@ import ( "fmt" "log/slog" + monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -37,6 +39,16 @@ const ( KindSecret = "Secret" ) +type commonMonitorSpec struct { + endpoints []monitoringv1.ScrapeEndpoint + mergedFromPod []monitoringv1.LabelMapping + mergedSelector metav1.LabelSelector + metadata *[]string + filterRunning *bool + limits *monitoringv1.ScrapeLimits + generatedSecrets []*unstructured.Unstructured +} + // ResourceConverter defines the interface for converting a specific Prometheus Operator resource kind. type ResourceConverter interface { // ImportKey returns the Kind of the resource this converter handles (e.g., "PodMonitor"). From 7717b1d680e4bc0af01d76cfeba4e3d5fec3386b Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Tue, 21 Jul 2026 18:08:08 +0000 Subject: [PATCH 02/16] refactor: create helper functions --- pkg/migrate/helpers.go | 61 +++++++++++++++++++++++++++++++++++++++ pkg/migrate/podmonitor.go | 38 ++++-------------------- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index ad0e31c16e..a1e7e60480 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -1031,3 +1031,64 @@ func buildClusterPodMonitoring( return u, nil } + +// resolveScrapeClass handles ScrapeClass resolution. +// Logs a warning that ScrapeClass settings will be lost. +// TODO(M2): Lookup ScrapeClass from Prometheus CR and merge its settings. +func resolveScrapeClass(name *string, logger *slog.Logger) { + if name != nil && *name != "" { + logger.Warn(fmt.Sprintf("ScrapeClass %q was not found in the inputs. The 'scrapeClassName' field has been dropped and inherited settings will be lost.", *name)) + } +} + +// validateScrapeProtocols logs a warning if any scrape protocol requires Protobuf. +func validateScrapeProtocols(protocols []pomonitoringv1.ScrapeProtocol, logger *slog.Logger) { + for _, sp := range protocols { + if sp == scrapeProtocolPrometheusProto || strings.Contains(strings.ToLower(string(sp)), "proto") { + logger.Warn("Scrape protocol settings (scrapeProtocols) requiring Protobuf are unsupported. Scrapes may fail if target lacks text fallback.") + break + } + } +} + +// resolveAttachMetadata appends "node" to metadata if attachMetadata.node is enabled. +func resolveAttachMetadata(attachMetadata *pomonitoringv1.AttachMetadata, baseMetadata *[]string, isCluster bool) *[]string { + if attachMetadata != nil && attachMetadata.Node != nil && *attachMetadata.Node { + if baseMetadata == nil { + if isCluster { + union := unionMetadata([]string{labelNode}, clusterMetadataDefaults) + return &union + } + return &[]string{labelNode} + } + if !slices.Contains(*baseMetadata, labelNode) { + metadataCopy := append(slices.Clone(*baseMetadata), labelNode) + return &metadataCopy + } + } + return baseMetadata +} + +// resolveFilterRunning evaluates filterRunning settings across endpoints and resolves them to a single resource-level setting. +func resolveFilterRunning(filterRunnings []*bool, logger *slog.Logger, isCluster bool) *bool { + var hasFalse, hasTrue bool + for _, fr := range filterRunnings { + if fr != nil && !*fr { + hasFalse = true + } else { + hasTrue = true + } + } + if hasFalse { + falseVal := false + if hasTrue { + if isCluster { + logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally on the ClusterPodMonitoring resource.") + } else { + logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally.") + } + } + return &falseVal + } + return nil +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index 8ae9a671de..ef6e5d5ea3 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -297,14 +297,8 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, // Spec-level warnings for unsupported fields. warnUnsupportedSpecFields(logger, &pm.Spec) - // TODO(M2): Resolve and merge ScrapeClass configurations from Prometheus CR if scrapeClassName is specified. - if pm.Spec.ScrapeClassName != nil && *pm.Spec.ScrapeClassName != "" { - logger.Warn(fmt.Sprintf("ScrapeClass %q was not found in the inputs. The 'scrapeClassName' field has been dropped and inherited settings will be lost.", *pm.Spec.ScrapeClassName)) - } - // Check against the PrometheusProto enum value. - if slices.Contains(pm.Spec.ScrapeProtocols, scrapeProtocolPrometheusProto) { - logger.Warn("Scrape protocol settings (scrapeProtocols) requiring Protobuf are unsupported. Scrapes may fail if target lacks text fallback.") - } + resolveScrapeClass(pm.Spec.ScrapeClassName, logger) + validateScrapeProtocols(pm.Spec.ScrapeProtocols, logger) var filteredMetadata *[]string if isCluster { @@ -312,15 +306,6 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, union := unionMetadata(*rules.ResourceCombined.Metadata, clusterMetadataDefaults) filteredMetadata = &union } - if pm.Spec.AttachMetadata != nil && pm.Spec.AttachMetadata.Node != nil && *pm.Spec.AttachMetadata.Node { - if filteredMetadata == nil { - union := unionMetadata([]string{labelNode}, clusterMetadataDefaults) - filteredMetadata = &union - } else { - union := unionMetadata([]string{labelNode}, *filteredMetadata) - filteredMetadata = &union - } - } } else { if rules.ResourceCombined.Metadata != nil { union := unionMetadata(*rules.ResourceCombined.Metadata, namespacedMetadataDefaults) @@ -349,24 +334,13 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, filteredMetadata = &union } } + filteredMetadata = resolveAttachMetadata(pm.Spec.AttachMetadata, filteredMetadata, isCluster) - var hasFalse, hasTrue bool + var filterRunnings []*bool for _, ep := range pm.Spec.PodMetricsEndpoints { - if ep.FilterRunning != nil && !*ep.FilterRunning { - hasFalse = true - } else { - hasTrue = true - } - } - // A nil filterRunning defaults to true in the GMP operator. - var filterRunning *bool - if hasFalse { - falseVal := false - filterRunning = &falseVal - if hasTrue { - logger.Warn("Endpoint-level configuration conflict detected: some endpoints are configured with 'filterRunning: false' and others with 'true' (or default), but GMP only supports 'filterRunning' at the resource level. Setting 'filterRunning: false' globally.") - } + filterRunnings = append(filterRunnings, ep.FilterRunning) } + filterRunning := resolveFilterRunning(filterRunnings, logger, isCluster) limits := convertLimits(pm.Spec.SampleLimit, pm.Spec.LabelLimit, pm.Spec.LabelNameLengthLimit, pm.Spec.LabelValueLengthLimit) From 5d3a96b13890b191d98915f7d17d27e153d84b28 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Tue, 21 Jul 2026 18:28:57 +0000 Subject: [PATCH 03/16] fix: add nil check on spec --- pkg/migrate/helpers.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index a1e7e60480..208d7906a6 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -16,6 +16,7 @@ package migrate import ( "encoding/base64" + "errors" "fmt" "log/slog" "slices" @@ -972,6 +973,9 @@ func buildPodMonitoring( spec *commonMonitorSpec, logger *slog.Logger, ) (*unstructured.Unstructured, error) { + if spec == nil { + return nil, errors.New("spec cannot be nil") + } gmpPM := &monitoringv1.PodMonitoring{ TypeMeta: BuildTypeMeta(KindPodMonitoring), ObjectMeta: CopyObjectMeta(srcMeta, targetNamespace, logger), @@ -1005,6 +1009,10 @@ func buildClusterPodMonitoring( spec *commonMonitorSpec, logger *slog.Logger, ) (*unstructured.Unstructured, error) { + if spec == nil { + return nil, errors.New("spec cannot be nil") + } + gmpCPM := &monitoringv1.ClusterPodMonitoring{ TypeMeta: BuildTypeMeta(KindClusterPodMonitoring), ObjectMeta: CopyObjectMeta(srcMeta, "", logger), From a5a261c646c2ca94e93c2e0d3de55252189a12dd Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Thu, 23 Jul 2026 16:07:43 +0000 Subject: [PATCH 04/16] refactor: create helpers for endpoint specific logic --- pkg/migrate/helpers.go | 137 ++++++++++++++++++++++++++++++++++++++ pkg/migrate/podmonitor.go | 118 +++++--------------------------- 2 files changed, 153 insertions(+), 102 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 208d7906a6..363a242116 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -24,6 +24,7 @@ import ( monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1" pomonitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + prommodel "github.com/prometheus/common/model" "github.com/prometheus/prometheus/google/export" "github.com/prometheus/prometheus/model/relabel" corev1 "k8s.io/api/core/v1" @@ -632,6 +633,62 @@ func (c *conversionContext) convertAuthorization(auth *pomonitoringv1.SafeAuthor }, nil } +// applyAuthAndTLS converts credentials and TLS settings for a generic endpoint. +func (c *conversionContext) applyAuthAndTLS( + i int, + gmpEp *monitoringv1.ScrapeEndpoint, + basicAuth *pomonitoringv1.BasicAuth, + oAuth2 *pomonitoringv1.OAuth2, + tlsConfig *pomonitoringv1.SafeTLSConfig, + authorization *pomonitoringv1.SafeAuthorization, + bearerTokenSecret corev1.SecretKeySelector, +) error { + if basicAuth != nil { + ba, err := c.convertBasicAuth(basicAuth) + if err != nil { + return fmt.Errorf("endpoint [%d]: basicAuth: %w", i, err) + } + gmpEp.BasicAuth = ba + } + if oAuth2 != nil { + oa, err := c.convertOAuth2(oAuth2) + if err != nil { + return fmt.Errorf("endpoint [%d]: oAuth2: %w", i, err) + } + gmpEp.OAuth2 = oa + } + if tlsConfig != nil { + tls, err := c.convertSafeTLSConfig(tlsConfig) + if err != nil { + return fmt.Errorf("endpoint [%d]: tlsConfig: %w", i, err) + } + gmpEp.TLS = tls + } + if authorization != nil { + auth, err := c.convertAuthorization(authorization) + if err != nil { + return fmt.Errorf("endpoint [%d]: authorization: %w", i, err) + } + gmpEp.Authorization = auth + } + + // Handle deprecated BearerTokenSecret -> Authorization. + if bearerTokenSecret.Name != "" { // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. + if gmpEp.Authorization != nil { + c.logger.Warn("Endpoint has both 'bearerTokenSecret' and 'authorization' defined. Dropping 'bearerTokenSecret'.", + slog.Int("endpoint_index", i)) + } else { + tokenSecret := bearerTokenSecret // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. + auth, err := c.convertAuthorization(&pomonitoringv1.SafeAuthorization{Credentials: &tokenSecret}) + if err != nil { + return fmt.Errorf("endpoint [%d]: bearerTokenSecret: %w", i, err) + } + gmpEp.Authorization = auth + } + } + return nil +} + func convertMetricRelabelings( logger *slog.Logger, configs []pomonitoringv1.RelabelConfig, @@ -1100,3 +1157,83 @@ func resolveFilterRunning(filterRunnings []*bool, logger *slog.Logger, isCluster } return nil } + +// resolveScrapeIntervalAndTimeout validates and caps timeout to interval if needed. +func resolveScrapeIntervalAndTimeout(logger *slog.Logger, interval, timeout string) (string, string, error) { + // TODO(M2): Inherit global scrape interval from Prometheus CR if empty. + if interval == "" { + logger.Warn("Scrape interval is empty. Defaulting to '30s' as GMP requires this field.") + interval = "30s" + } + + intDur, err := prommodel.ParseDuration(interval) + if err != nil { + return "", "", fmt.Errorf("invalid interval %q: %w", interval, err) + } + + // TODO(M2): Inherit global scrape timeout from Prometheus CR if empty. + if timeout != "" { + toDur, err := prommodel.ParseDuration(timeout) + if err != nil { + return "", "", fmt.Errorf("invalid scrapeTimeout %q: %w", timeout, err) + } + if toDur > intDur { + logger.Warn("Scrape timeout is larger than scrape interval. Capping timeout to interval.", + slog.String("timeout", timeout), + slog.String("interval", interval)) + timeout = interval + } + } + return interval, timeout, nil +} + +// convertProxyURL verifies proxy URL credentials. +func convertProxyURL(proxyURL *string) (string, error) { + if proxyURL == nil { + return "", nil + } + if strings.Contains(*proxyURL, "@") { + return "", errors.New("proxyUrl contains credentials (matches '@'), which is blocked by GMP API validation") + } + return *proxyURL, nil +} + +// warnUnsupportedEndpointFields logs warnings for fields that GMP does not support. +func warnUnsupportedEndpointFields(logger *slog.Logger, followRedirects *bool, enableHTTP2 *bool, honorLabels bool, honorTimestamps *bool, trackTimestampsStaleness *bool, i int) { + if followRedirects != nil && !*followRedirects { + logger.Warn(fmt.Sprintf("endpoint [%d]: field 'followRedirects: false' is unsupported by GMP Managed Collection and has been dropped. The collector will always follow redirects.", i)) + } + if enableHTTP2 != nil && !*enableHTTP2 { + logger.Warn(fmt.Sprintf("endpoint [%d]: field 'enableHttp2: false' is unsupported by GMP Managed Collection and has been dropped. The collector will always negotiate HTTP/2 for TLS connections.", i)) + } + if honorLabels { + logger.Warn(fmt.Sprintf("endpoint [%d]: field 'honorLabels: true' is unsupported and dropped. GMP always overrides conflicting labels. Clashing metric labels will be renamed with the 'exported_' prefix.", i)) + } + if honorTimestamps != nil && *honorTimestamps { + logger.Warn(fmt.Sprintf("endpoint [%d]: field 'honorTimestamps: true' is unsupported and dropped. GMP always uses the scrape ingestion timestamp. Target metric timestamps will be ignored.", i)) + } + if trackTimestampsStaleness != nil { + logger.Warn(fmt.Sprintf("endpoint [%d]: fField 'trackTimestampsStaleness' is unsupported in GMP and has been dropped.", i)) + } +} + +// combineAndConvertRelabelings combines promoted pre-scrape rules and converts metricRelabelings. +func combineAndConvertRelabelings(logger *slog.Logger, promoted []monitoringv1.RelabelingRule, configs []pomonitoringv1.RelabelConfig) ([]monitoringv1.RelabelingRule, error) { + totalRules := len(promoted) + len(configs) + if totalRules == 0 { + return nil, nil + } + + allRules := make([]monitoringv1.RelabelingRule, 0, totalRules) + allRules = append(allRules, promoted...) + + if len(configs) > 0 { + rules, err := convertMetricRelabelings(logger, configs) + if err != nil { + return nil, err + } + allRules = append(allRules, rules...) + } + + return allRules, nil +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index ef6e5d5ea3..7d2f36f039 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -24,7 +24,6 @@ import ( monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1" pomonitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" - prommodel "github.com/prometheus/common/model" "github.com/prometheus/prometheus/google/export" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -147,123 +146,38 @@ func (c *PodMonitorConverter) convertEndpoints( gmpEp.Params = ep.Params // 3. Scrape Intervals & Timeouts. - gmpEp.Interval = string(ep.Interval) - gmpEp.Timeout = string(ep.ScrapeTimeout) - - // TODO(M2): Inherit global scrape interval from Prometheus CR if empty. - if gmpEp.Interval == "" { - convCtx.logger.Warn("Scrape interval is empty. Defaulting to '30s' as GMP requires this field.") - gmpEp.Interval = "30s" - } - - intDur, err := prommodel.ParseDuration(gmpEp.Interval) + interval, timeout, err := resolveScrapeIntervalAndTimeout(convCtx.logger, string(ep.Interval), string(ep.ScrapeTimeout)) if err != nil { - return nil, fmt.Errorf("endpoint [%d]: invalid interval %q: %w", i, gmpEp.Interval, err) - } - - if gmpEp.Timeout != "" { - toDur, err := prommodel.ParseDuration(gmpEp.Timeout) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: invalid scrapeTimeout %q: %w", i, gmpEp.Timeout, err) - } - if toDur > intDur { - convCtx.logger.Warn("Scrape timeout is larger than scrape interval. Capping timeout to interval.", - slog.String("timeout", gmpEp.Timeout), - slog.String("interval", gmpEp.Interval)) - gmpEp.Timeout = gmpEp.Interval - } + return nil, fmt.Errorf("endpoint [%d]: %w", i, err) } - // TODO(M2): Inherit global scrape timeout from Prometheus CR if empty. + gmpEp.Interval = interval + gmpEp.Timeout = timeout // 4. Relabeling Rules (Promoted Pre-Scrape + MetricRelabelings). - totalRules := len(epResults[i].PromotedRules) + len(ep.MetricRelabelConfigs) - var allRules []monitoringv1.RelabelingRule - if totalRules > 0 { - allRules = make([]monitoringv1.RelabelingRule, 0, totalRules) - allRules = append(allRules, epResults[i].PromotedRules...) - if len(ep.MetricRelabelConfigs) > 0 { - rules, err := convertMetricRelabelings(convCtx.logger, ep.MetricRelabelConfigs) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } - allRules = append(allRules, rules...) - } + relabelings, err := combineAndConvertRelabelings(convCtx.logger, epResults[i].PromotedRules, ep.MetricRelabelConfigs) + if err != nil { + return nil, fmt.Errorf("endpoint [%d]: %w", i, err) } - gmpEp.MetricRelabeling = allRules + gmpEp.MetricRelabeling = relabelings // Proxy Settings. - if ep.ProxyURL != nil { - if strings.Contains(*ep.ProxyURL, "@") { - return nil, fmt.Errorf("endpoint [%d]: proxyUrl contains credentials (matches '@'), which is blocked by GMP API validation", i) - } - gmpEp.ProxyURL = *ep.ProxyURL + proxyURL, err := convertProxyURL(ep.ProxyURL) + if err != nil { + return nil, fmt.Errorf("endpoint [%d]: %w", i, err) } + gmpEp.ProxyURL = proxyURL // noProxy, proxyConnectHeader, and proxyFromEnvironment fields are silently dropped. // The pinned Prometheus Operator version lacks these fields, and GMP does not support them anyway. // Auth & TLS mappings. - if ep.BasicAuth != nil { - ba, err := convCtx.convertBasicAuth(ep.BasicAuth) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: basicAuth: %w", i, err) - } - gmpEp.BasicAuth = ba - } - if ep.OAuth2 != nil { - oa, err := convCtx.convertOAuth2(ep.OAuth2) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: oAuth2: %w", i, err) - } - gmpEp.OAuth2 = oa - } - if ep.TLSConfig != nil { - tls, err := convCtx.convertSafeTLSConfig(ep.TLSConfig) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: tlsConfig: %w", i, err) - } - gmpEp.TLS = tls - } - if ep.Authorization != nil { - auth, err := convCtx.convertAuthorization(ep.Authorization) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: authorization: %w", i, err) - } - gmpEp.Authorization = auth - } - - // Handle deprecated BearerTokenSecret -> Authorization. - if ep.BearerTokenSecret.Name != "" { // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. - if gmpEp.Authorization != nil { - convCtx.logger.Warn("Endpoint has both 'bearerTokenSecret' and 'authorization' defined. Dropping 'bearerTokenSecret'.", - slog.Int("endpoint_index", i)) - } else { - tokenSecret := ep.BearerTokenSecret // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. - auth, err := convCtx.convertAuthorization(&pomonitoringv1.SafeAuthorization{Credentials: &tokenSecret}) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: bearerTokenSecret: %w", i, err) - } - gmpEp.Authorization = auth - } + err = convCtx.applyAuthAndTLS(i, &gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) + if err != nil { + return nil, err } // 5. Warnings for Unsupported Fields in Endpoint. - if ep.FollowRedirects != nil && !*ep.FollowRedirects { - convCtx.logger.Warn(fmt.Sprintf("endpoint [%d]: field 'followRedirects: false' is unsupported by GMP Managed Collection and has been dropped. The collector will always follow redirects.", i)) - } - if ep.EnableHttp2 != nil && !*ep.EnableHttp2 { - convCtx.logger.Warn(fmt.Sprintf("endpoint [%d]: field 'enableHttp2: false' is unsupported by GMP Managed Collection and has been dropped. The collector will always negotiate HTTP/2 for TLS connections.", i)) - } - - if ep.HonorLabels { - convCtx.logger.Warn("Field 'honorLabels: true' is unsupported and dropped. GMP always overrides conflicting labels. Clashing metric labels will be renamed with the 'exported_' prefix.") - } - if ep.HonorTimestamps != nil && *ep.HonorTimestamps { - convCtx.logger.Warn("Field 'honorTimestamps: true' is unsupported and dropped. GMP always uses the scrape ingestion timestamp. Target metric timestamps will be ignored.") - } - if ep.TrackTimestampsStaleness != nil { - convCtx.logger.Warn("Field 'trackTimestampsStaleness' is unsupported in GMP and has been dropped.") - } + warnUnsupportedEndpointFields(convCtx.logger, ep.FollowRedirects, ep.EnableHttp2, ep.HonorLabels, ep.HonorTimestamps, ep.TrackTimestampsStaleness, i) gmpEndpoints = append(gmpEndpoints, gmpEp) } From 6603d6bed6f44160a68d145810cd0524d773abd0 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Thu, 23 Jul 2026 16:13:16 +0000 Subject: [PATCH 05/16] fix: resolve lint errors --- pkg/migrate/helpers.go | 2 +- pkg/migrate/podmonitor.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 363a242116..0290ab1a5c 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -1159,7 +1159,7 @@ func resolveFilterRunning(filterRunnings []*bool, logger *slog.Logger, isCluster } // resolveScrapeIntervalAndTimeout validates and caps timeout to interval if needed. -func resolveScrapeIntervalAndTimeout(logger *slog.Logger, interval, timeout string) (string, string, error) { +func resolveScrapeIntervalAndTimeout(logger *slog.Logger, interval, timeout string) (resolvedInterval, resolvedTimeout string, err error) { // TODO(M2): Inherit global scrape interval from Prometheus CR if empty. if interval == "" { logger.Warn("Scrape interval is empty. Defaulting to '30s' as GMP requires this field.") diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index 7d2f36f039..ac78f595f6 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -171,7 +171,7 @@ func (c *PodMonitorConverter) convertEndpoints( // The pinned Prometheus Operator version lacks these fields, and GMP does not support them anyway. // Auth & TLS mappings. - err = convCtx.applyAuthAndTLS(i, &gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) + err = convCtx.applyAuthAndTLS(i, &gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. if err != nil { return nil, err } From 69fe2a316bf2ce131da5ee4ca6cb55533b502c48 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Thu, 23 Jul 2026 18:32:59 +0000 Subject: [PATCH 06/16] refactor: namespace and relabeling refactor --- pkg/migrate/helpers.go | 40 +++++++++++++++++++++++++---- pkg/migrate/podmonitor.go | 53 +++++++++++++-------------------------- 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 0290ab1a5c..71d3be0260 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -16,6 +16,7 @@ package migrate import ( "encoding/base64" + "encoding/json" "errors" "fmt" "log/slog" @@ -30,7 +31,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation" ) @@ -125,8 +125,8 @@ func CopyObjectMeta(src metav1.ObjectMeta, targetNamespace string, logger *slog. return dst } -// ParseAndCleanNamespaces trims whitespace, filters out empty strings, and deduplicates namespaces. -func ParseAndCleanNamespaces(namespaces []string) []string { +// parseAndCleanNamespaces trims whitespace, filters out empty strings, and deduplicates namespaces. +func parseAndCleanNamespaces(namespaces []string) []string { unique := make(map[string]bool) var cleaned []string for _, ns := range namespaces { @@ -139,6 +139,21 @@ func ParseAndCleanNamespaces(namespaces []string) []string { return cleaned } +// determineNamespaceScoping resolves the target namespaces from a NamespaceSelector. +func determineNamespaceScoping(nsSel pomonitoringv1.NamespaceSelector, defaultNS string) ([]string, bool, error) { + if nsSel.Any { + return nil, true, nil + } + if len(nsSel.MatchNames) > 0 { + targetNamespaces := parseAndCleanNamespaces(nsSel.MatchNames) + if len(targetNamespaces) == 0 { + return nil, false, errors.New("namespaceSelector.matchNames contains only empty or invalid values") + } + return targetNamespaces, false, nil + } + return []string{defaultNS}, false, nil +} + // conversionContext groups common parameters passed down to conversion helper functions. type conversionContext struct { logger *slog.Logger @@ -1023,6 +1038,21 @@ func convertLimits(sampleLimit, labelLimit, labelNameLengthLimit, labelValueLeng return limits } +// toStrictUnstructured converts a struct to a strictly JSON-compatible unstructured map. +// uses JSON to silently convert unsupported Go primitives (like uint64) into safe float64 numbers. +// ensures the resulting map will not panic on DeepCopy. +func toStrictUnstructured(obj interface{}) (map[string]interface{}, error) { + b, err := json.Marshal(obj) + if err != nil { + return nil, err + } + var u map[string]interface{} + if err := json.Unmarshal(b, &u); err != nil { + return nil, err + } + return u, nil +} + // buildPodMonitoring constructs a GMP PodMonitoring resource from common spec. func buildPodMonitoring( srcMeta metav1.ObjectMeta, @@ -1048,7 +1078,7 @@ func buildPodMonitoring( }, } - unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpPM) + unstructuredMap, err := toStrictUnstructured(gmpPM) if err != nil { return nil, fmt.Errorf("failed to marshal PodMonitoring: %w", err) } @@ -1085,7 +1115,7 @@ func buildClusterPodMonitoring( }, } - unstructuredMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(gmpCPM) + unstructuredMap, err := toStrictUnstructured(gmpCPM) if err != nil { return nil, fmt.Errorf("failed to marshal ClusterPodMonitoring: %w", err) } diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index ac78f595f6..8e12c5a3c1 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -61,10 +61,12 @@ func (c *PodMonitorConverter) Convert(_ context.Context, logger *slog.Logger, un // TODO(M2): Override local namespace scoping if Prometheus CR specifies ignoreNamespaceSelectors. // 2. Determine Scoping based on namespaceSelector. - nsSel := podMonitor.Spec.NamespaceSelector + targetNamespaces, isClusterScoped, err := determineNamespaceScoping(podMonitor.Spec.NamespaceSelector, podMonitor.Namespace) + if err != nil { + return nil, err + } - if nsSel.Any { - // Case A: namespaceSelector.any = true -> Single ClusterPodMonitoring. + if isClusterScoped { logger.Info("namespaceSelector selects 'any: true'. Translated to 'ClusterPodMonitoring'") u, generatedSecrets, err := c.convertToClusterPodMonitoring(&podMonitor, logger, cache) if err != nil { @@ -75,44 +77,23 @@ func (c *PodMonitorConverter) Convert(_ context.Context, logger *slog.Logger, un return outputs, nil } - if len(nsSel.MatchNames) > 0 { - // Case B: namespaceSelector.matchNames listed -> Multiple PodMonitoring resources (one per namespace). - targetNamespaces := ParseAndCleanNamespaces(nsSel.MatchNames) - - // 2.1 Fail if all provided names were empty/whitespace (broken config). - if len(targetNamespaces) == 0 { - return nil, errors.New("namespaceSelector.matchNames contains only empty or invalid values") - } - - if len(targetNamespaces) > 1 { - logger.Info("namespaceSelector targets multiple namespaces. Generating separate PodMonitoring resources for each namespace", - slog.Any("namespaces", targetNamespaces), - ) - } - - // 2.2 Convert to a base namespaced PodMonitoring. - baseU, generatedSecrets, err := c.convertToPodMonitoring(&podMonitor, logger, cache) - if err != nil { - return nil, err - } - - // 2.3 Clone and apply target namespaces. - var outputs []*unstructured.Unstructured - for _, ns := range targetNamespaces { - uClone := baseU.DeepCopy() - uClone.SetNamespace(ns) - outputs = append(outputs, uClone) - } - outputs = append(outputs, generatedSecrets...) - return outputs, nil + if len(targetNamespaces) > 1 { + logger.Info("namespaceSelector targets multiple namespaces. Generating separate PodMonitoring resources for each namespace", + slog.Any("namespaces", targetNamespaces), + ) } - // Case C: namespaceSelector is empty/omitted -> Single PodMonitoring in local namespace. - u, generatedSecrets, err := c.convertToPodMonitoring(&podMonitor, logger, cache) + baseU, generatedSecrets, err := c.convertToPodMonitoring(&podMonitor, logger, cache) if err != nil { return nil, err } - outputs := []*unstructured.Unstructured{u} + + var outputs []*unstructured.Unstructured + for _, ns := range targetNamespaces { + uClone := baseU.DeepCopy() + uClone.SetNamespace(ns) + outputs = append(outputs, uClone) + } outputs = append(outputs, generatedSecrets...) return outputs, nil } From a4934b38d5fc5fc2cffda6cdf9af1454fa50a932 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Thu, 23 Jul 2026 18:42:51 +0000 Subject: [PATCH 07/16] fix: resolve lint errors and nil pointer --- pkg/migrate/helpers.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 71d3be0260..70947bfd50 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -658,6 +658,10 @@ func (c *conversionContext) applyAuthAndTLS( authorization *pomonitoringv1.SafeAuthorization, bearerTokenSecret corev1.SecretKeySelector, ) error { + if gmpEp == nil { + return errors.New("scrape endpoint cannot be nil") + } + if basicAuth != nil { ba, err := c.convertBasicAuth(basicAuth) if err != nil { @@ -1041,12 +1045,12 @@ func convertLimits(sampleLimit, labelLimit, labelNameLengthLimit, labelValueLeng // toStrictUnstructured converts a struct to a strictly JSON-compatible unstructured map. // uses JSON to silently convert unsupported Go primitives (like uint64) into safe float64 numbers. // ensures the resulting map will not panic on DeepCopy. -func toStrictUnstructured(obj interface{}) (map[string]interface{}, error) { +func toStrictUnstructured(obj any) (map[string]any, error) { b, err := json.Marshal(obj) if err != nil { return nil, err } - var u map[string]interface{} + var u map[string]any if err := json.Unmarshal(b, &u); err != nil { return nil, err } From 5203adfd0563dcd614f57a0a8207e1abe75362b4 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Tue, 28 Jul 2026 15:16:16 +0000 Subject: [PATCH 08/16] fix: rebase changes --- pkg/migrate/podmonitor.go | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index 8e12c5a3c1..e31865367b 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -256,7 +256,26 @@ func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonit return nil, nil, err } - u, err := buildPodMonitoring(pm.ObjectMeta, pm.Namespace, res, logger) + var filteredMetadata *[]string + if res.metadata != nil { + union := unionMetadata(*res.metadata, namespacedMetadataDefaults) + var md []string + for _, m := range union { + if m != export.KeyNamespace { + md = append(md, m) + } else { + logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The rule has been dropped.") + } + } + if len(md) > 0 { + filteredMetadata = &md + } + } + + resCopy := *res + resCopy.metadata = filteredMetadata + + u, err := buildPodMonitoring(pm.ObjectMeta, pm.Namespace, &resCopy, logger) if err != nil { return nil, nil, err } @@ -270,7 +289,16 @@ func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.P return nil, nil, err } - u, err := buildClusterPodMonitoring(pm.ObjectMeta, res, logger) + var filteredMetadata *[]string + if res.metadata != nil { + union := unionMetadata(*res.metadata, clusterMetadataDefaults) + filteredMetadata = &union + } + + resCopy := *res + resCopy.metadata = filteredMetadata + + u, err := buildClusterPodMonitoring(pm.ObjectMeta, &resCopy, logger) if err != nil { return nil, nil, err } From bafaf79f8f996f2a246d2ee7d606645beb6471cf Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Thu, 30 Jul 2026 21:12:02 +0000 Subject: [PATCH 09/16] fix: resolve bugs and improve test coverage --- pkg/migrate/helpers.go | 59 ++++--- pkg/migrate/helpers_test.go | 318 ++++++++++++++++++++++++++++++++++++ pkg/migrate/podmonitor.go | 95 ++++++----- 3 files changed, 401 insertions(+), 71 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 70947bfd50..33000bb8d5 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -16,7 +16,6 @@ package migrate import ( "encoding/base64" - "encoding/json" "errors" "fmt" "log/slog" @@ -31,6 +30,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation" ) @@ -650,7 +650,6 @@ func (c *conversionContext) convertAuthorization(auth *pomonitoringv1.SafeAuthor // applyAuthAndTLS converts credentials and TLS settings for a generic endpoint. func (c *conversionContext) applyAuthAndTLS( - i int, gmpEp *monitoringv1.ScrapeEndpoint, basicAuth *pomonitoringv1.BasicAuth, oAuth2 *pomonitoringv1.OAuth2, @@ -665,28 +664,28 @@ func (c *conversionContext) applyAuthAndTLS( if basicAuth != nil { ba, err := c.convertBasicAuth(basicAuth) if err != nil { - return fmt.Errorf("endpoint [%d]: basicAuth: %w", i, err) + return fmt.Errorf("basicAuth: %w", err) } gmpEp.BasicAuth = ba } if oAuth2 != nil { oa, err := c.convertOAuth2(oAuth2) if err != nil { - return fmt.Errorf("endpoint [%d]: oAuth2: %w", i, err) + return fmt.Errorf("oAuth2: %w", err) } gmpEp.OAuth2 = oa } if tlsConfig != nil { tls, err := c.convertSafeTLSConfig(tlsConfig) if err != nil { - return fmt.Errorf("endpoint [%d]: tlsConfig: %w", i, err) + return fmt.Errorf("tlsConfig: %w", err) } gmpEp.TLS = tls } if authorization != nil { auth, err := c.convertAuthorization(authorization) if err != nil { - return fmt.Errorf("endpoint [%d]: authorization: %w", i, err) + return fmt.Errorf("authorization: %w", err) } gmpEp.Authorization = auth } @@ -694,13 +693,12 @@ func (c *conversionContext) applyAuthAndTLS( // Handle deprecated BearerTokenSecret -> Authorization. if bearerTokenSecret.Name != "" { // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. if gmpEp.Authorization != nil { - c.logger.Warn("Endpoint has both 'bearerTokenSecret' and 'authorization' defined. Dropping 'bearerTokenSecret'.", - slog.Int("endpoint_index", i)) + c.logger.Warn("Endpoint has both 'bearerTokenSecret' and 'authorization' defined. Dropping 'bearerTokenSecret'.") } else { tokenSecret := bearerTokenSecret // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. auth, err := c.convertAuthorization(&pomonitoringv1.SafeAuthorization{Credentials: &tokenSecret}) if err != nil { - return fmt.Errorf("endpoint [%d]: bearerTokenSecret: %w", i, err) + return fmt.Errorf("bearerTokenSecret: %w", err) } gmpEp.Authorization = auth } @@ -711,7 +709,7 @@ func (c *conversionContext) applyAuthAndTLS( func convertMetricRelabelings( logger *slog.Logger, configs []pomonitoringv1.RelabelConfig, -) ([]monitoringv1.RelabelingRule, error) { +) []monitoringv1.RelabelingRule { var rules []monitoringv1.RelabelingRule for _, config := range configs { @@ -759,7 +757,7 @@ func convertMetricRelabelings( rules = append(rules, rule) } - return rules, nil + return rules } func convertTargetLabels(logger *slog.Logger, sourceLabels []string, jobLabel string, labelKind string) []monitoringv1.LabelMapping { @@ -1042,19 +1040,31 @@ func convertLimits(sampleLimit, labelLimit, labelNameLengthLimit, labelValueLeng return limits } -// toStrictUnstructured converts a struct to a strictly JSON-compatible unstructured map. -// uses JSON to silently convert unsupported Go primitives (like uint64) into safe float64 numbers. -// ensures the resulting map will not panic on DeepCopy. +// toStrictUnstructured converts a struct to an unstructured map and normalizes uint64 fields to int64. +// This prevents unstructured.DeepCopy from panicking without losing integer precision. func toStrictUnstructured(obj any) (map[string]any, error) { - b, err := json.Marshal(obj) + u, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) if err != nil { return nil, err } - var u map[string]any - if err := json.Unmarshal(b, &u); err != nil { - return nil, err + return sanitizeUInt64(u).(map[string]any), nil +} + +// sanitizeUInt64 recursively converts uint64 primitives to int64 for unstructured compatibility. +func sanitizeUInt64(val any) any { + switch v := val.(type) { + case uint64: + return int64(v) + case map[string]any: + for k, child := range v { + v[k] = sanitizeUInt64(child) + } + case []any: + for i, child := range v { + v[i] = sanitizeUInt64(child) + } } - return u, nil + return val } // buildPodMonitoring constructs a GMP PodMonitoring resource from common spec. @@ -1252,22 +1262,19 @@ func warnUnsupportedEndpointFields(logger *slog.Logger, followRedirects *bool, e } // combineAndConvertRelabelings combines promoted pre-scrape rules and converts metricRelabelings. -func combineAndConvertRelabelings(logger *slog.Logger, promoted []monitoringv1.RelabelingRule, configs []pomonitoringv1.RelabelConfig) ([]monitoringv1.RelabelingRule, error) { +func combineAndConvertRelabelings(logger *slog.Logger, promoted []monitoringv1.RelabelingRule, configs []pomonitoringv1.RelabelConfig) []monitoringv1.RelabelingRule { totalRules := len(promoted) + len(configs) if totalRules == 0 { - return nil, nil + return nil } allRules := make([]monitoringv1.RelabelingRule, 0, totalRules) allRules = append(allRules, promoted...) if len(configs) > 0 { - rules, err := convertMetricRelabelings(logger, configs) - if err != nil { - return nil, err - } + rules := convertMetricRelabelings(logger, configs) allRules = append(allRules, rules...) } - return allRules, nil + return allRules } diff --git a/pkg/migrate/helpers_test.go b/pkg/migrate/helpers_test.go index 7a3f07a20c..c9e9353010 100644 --- a/pkg/migrate/helpers_test.go +++ b/pkg/migrate/helpers_test.go @@ -597,3 +597,321 @@ func TestConvertLimits(t *testing.T) { }) } } + +func TestDetermineNamespaceScoping(t *testing.T) { + tests := []struct { + name string + nsSel pomonitoringv1.NamespaceSelector + defaultNS string + expectedNS []string + expectedIsCluster bool + expectErr bool + }{ + { + name: "any namespace true", + nsSel: pomonitoringv1.NamespaceSelector{Any: true}, + defaultNS: "default", + expectedNS: nil, + expectedIsCluster: true, + expectErr: false, + }, + { + name: "specific matchNames", + nsSel: pomonitoringv1.NamespaceSelector{MatchNames: []string{"ns1", "ns2"}}, + defaultNS: "default", + expectedNS: []string{"ns1", "ns2"}, + expectedIsCluster: false, + expectErr: false, + }, + { + name: "empty matchNames fallback to default", + nsSel: pomonitoringv1.NamespaceSelector{}, + defaultNS: "default", + expectedNS: []string{"default"}, + expectedIsCluster: false, + expectErr: false, + }, + { + name: "invalid matchNames", + nsSel: pomonitoringv1.NamespaceSelector{MatchNames: []string{""}}, + defaultNS: "default", + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ns, isCluster, err := determineNamespaceScoping(tc.nsSel, tc.defaultNS) + if (err != nil) != tc.expectErr { + t.Fatalf("determineNamespaceScoping() error = %v, expectErr = %v", err, tc.expectErr) + } + if tc.expectErr { + return + } + if isCluster != tc.expectedIsCluster { + t.Errorf("determineNamespaceScoping() isCluster = %v, want %v", isCluster, tc.expectedIsCluster) + } + if diff := cmp.Diff(tc.expectedNS, ns); diff != "" { + t.Errorf("determineNamespaceScoping() namespaces mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestResolveScrapeIntervalAndTimeout(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + tests := []struct { + name string + interval string + timeout string + expectedInt string + expectedTimeout string + expectErr bool + }{ + { + name: "empty defaults to 30s", + interval: "", + timeout: "", + expectedInt: "30s", + expectedTimeout: "", + expectErr: false, + }, + { + name: "valid interval and timeout", + interval: "15s", + timeout: "10s", + expectedInt: "15s", + expectedTimeout: "10s", + expectErr: false, + }, + { + name: "timeout larger than interval is capped", + interval: "10s", + timeout: "20s", + expectedInt: "10s", + expectedTimeout: "10s", + expectErr: false, + }, + { + name: "invalid interval duration", + interval: "invalid", + expectErr: true, + }, + { + name: "invalid timeout duration", + interval: "15s", + timeout: "invalid", + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + intVal, toVal, err := resolveScrapeIntervalAndTimeout(logger, tc.interval, tc.timeout) + if (err != nil) != tc.expectErr { + t.Fatalf("resolveScrapeIntervalAndTimeout() error = %v, expectErr = %v", err, tc.expectErr) + } + if tc.expectErr { + return + } + if intVal != tc.expectedInt { + t.Errorf("resolveScrapeIntervalAndTimeout() interval = %v, want %v", intVal, tc.expectedInt) + } + if toVal != tc.expectedTimeout { + t.Errorf("resolveScrapeIntervalAndTimeout() timeout = %v, want %v", toVal, tc.expectedTimeout) + } + }) + } +} + +func TestConvertProxyURL(t *testing.T) { + tests := []struct { + name string + proxyURL *string + expectedURL string + expectErr bool + }{ + { + name: "nil proxyURL", + proxyURL: nil, + expectedURL: "", + expectErr: false, + }, + { + name: "valid proxyURL without credentials", + proxyURL: ptrTo("http://proxy.example.com"), + expectedURL: "http://proxy.example.com", + expectErr: false, + }, + { + name: "proxyURL with credentials returns error", + proxyURL: ptrTo("http://user:pass@proxy.example.com"), + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + url, err := convertProxyURL(tc.proxyURL) + if (err != nil) != tc.expectErr { + t.Fatalf("convertProxyURL() error = %v, expectErr = %v", err, tc.expectErr) + } + if tc.expectErr { + return + } + if url != tc.expectedURL { + t.Errorf("convertProxyURL() = %v, want %v", url, tc.expectedURL) + } + }) + } +} + +func TestResolveFilterRunning(t *testing.T) { + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + tests := []struct { + name string + filterRunnings []*bool + isCluster bool + expected *bool + }{ + { + name: "empty cluster monitor defaults to nil", + filterRunnings: nil, + isCluster: true, + expected: nil, + }, + { + name: "empty namespaced monitor defaults to nil", + filterRunnings: nil, + isCluster: false, + expected: nil, + }, + { + name: "all true resolves to nil (GMP default)", + filterRunnings: []*bool{ptrTo(true), ptrTo(true)}, + isCluster: false, + expected: nil, + }, + { + name: "all false resolves to false", + filterRunnings: []*bool{ptrTo(false), ptrTo(false)}, + isCluster: false, + expected: ptrTo(false), + }, + { + name: "mixed true and false resolves to false", + filterRunnings: []*bool{ptrTo(true), ptrTo(false)}, + isCluster: false, + expected: ptrTo(false), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := resolveFilterRunning(tc.filterRunnings, logger, tc.isCluster) + if diff := cmp.Diff(tc.expected, got); diff != "" { + t.Errorf("resolveFilterRunning() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestResolveAttachMetadata(t *testing.T) { + tests := []struct { + name string + attachMetadata *pomonitoringv1.AttachMetadata + base *[]string + isCluster bool + expected *[]string + }{ + { + name: "nil attachMetadata returns base", + attachMetadata: nil, + base: &[]string{"pod"}, + isCluster: false, + expected: &[]string{"pod"}, + }, + { + name: "attachMetadata node false returns base", + attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(false)}, + base: nil, + isCluster: false, + expected: nil, + }, + { + name: "namespaced monitor with nil base returns node", + attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, + base: nil, + isCluster: false, + expected: &[]string{"node"}, + }, + { + name: "cluster monitor with nil base returns node plus cluster defaults", + attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, + base: nil, + isCluster: true, + expected: &[]string{"container", "namespace", "node", "pod", "top_level_controller_name", "top_level_controller_type"}, + }, + { + name: "namespaced monitor with existing base appends node", + attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, + base: &[]string{"pod"}, + isCluster: false, + expected: &[]string{"pod", "node"}, + }, + { + name: "namespaced monitor with node already present does not duplicate", + attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, + base: &[]string{"pod", "node"}, + isCluster: false, + expected: &[]string{"pod", "node"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := resolveAttachMetadata(tc.attachMetadata, tc.base, tc.isCluster) + if diff := cmp.Diff(tc.expected, got); diff != "" { + t.Errorf("resolveAttachMetadata() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestToStrictUnstructured(t *testing.T) { + tests := []struct { + name string + obj any + expectErr bool + }{ + { + name: "scrape limits with uint64 converts to int64", + obj: &monitoringv1.ScrapeLimits{ + Samples: 5000, + Labels: 100, + }, + expectErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + u, err := toStrictUnstructured(tc.obj) + if (err != nil) != tc.expectErr { + t.Fatalf("toStrictUnstructured() error = %v, expectErr = %v", err, tc.expectErr) + } + if tc.expectErr { + return + } + // Verify uint64 fields are converted to int64. + if val, ok := u["samples"]; ok { + if _, isInt64 := val.(int64); !isInt64 { + t.Errorf("expected samples to be int64, got %T", val) + } + } + // Verify DeepCopy does not panic. + unstruct := &unstructured.Unstructured{Object: u} + _ = unstruct.DeepCopy() + }) + } +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index e31865367b..0c02885aa7 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -135,11 +135,7 @@ func (c *PodMonitorConverter) convertEndpoints( gmpEp.Timeout = timeout // 4. Relabeling Rules (Promoted Pre-Scrape + MetricRelabelings). - relabelings, err := combineAndConvertRelabelings(convCtx.logger, epResults[i].PromotedRules, ep.MetricRelabelConfigs) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } - gmpEp.MetricRelabeling = relabelings + gmpEp.MetricRelabeling = combineAndConvertRelabelings(convCtx.logger, epResults[i].PromotedRules, ep.MetricRelabelConfigs) // Proxy Settings. proxyURL, err := convertProxyURL(ep.ProxyURL) @@ -152,9 +148,9 @@ func (c *PodMonitorConverter) convertEndpoints( // The pinned Prometheus Operator version lacks these fields, and GMP does not support them anyway. // Auth & TLS mappings. - err = convCtx.applyAuthAndTLS(i, &gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. + err = convCtx.applyAuthAndTLS(&gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. if err != nil { - return nil, err + return nil, fmt.Errorf("endpoint [%d]: %w", i, err) } // 5. Warnings for Unsupported Fields in Endpoint. @@ -187,7 +183,11 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, return nil, err } if len(mergedSelector.MatchLabels) == 0 && len(mergedSelector.MatchExpressions) == 0 { - logger.Warn("Resulting PodMonitoring selector is empty. It will select and scrape all pods in this namespace. Verify if this is intended.") + if isCluster { + logger.Warn("Resulting ClusterPodMonitoring selector is empty. It will select and scrape all pods across all namespaces. Verify if this is intended.") + } else { + logger.Warn("Resulting PodMonitoring selector is empty. It will select and scrape all pods in this namespace. Verify if this is intended.") + } } // Spec-level warnings for unsupported fields. @@ -250,55 +250,52 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, }, nil } -func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - res, err := c.convertMonitorSpec(pm, logger, cache, false) - if err != nil { - return nil, nil, err +// filterMetadata applies namespaced or cluster metadata defaults and strips namespace metadata in namespaced resources. +func filterMetadata(metadata *[]string, isCluster bool, logger *slog.Logger) *[]string { + if metadata == nil { + return nil } - - var filteredMetadata *[]string - if res.metadata != nil { - union := unionMetadata(*res.metadata, namespacedMetadataDefaults) - var md []string - for _, m := range union { - if m != export.KeyNamespace { - md = append(md, m) - } else { - logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The rule has been dropped.") - } - } - if len(md) > 0 { - filteredMetadata = &md + if isCluster { + union := unionMetadata(*metadata, clusterMetadataDefaults) + return &union + } + union := unionMetadata(*metadata, namespacedMetadataDefaults) + var md []string + for _, m := range union { + if m != export.KeyNamespace { + md = append(md, m) + } else { + logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The rule has been dropped.") } } - - resCopy := *res - resCopy.metadata = filteredMetadata - - u, err := buildPodMonitoring(pm.ObjectMeta, pm.Namespace, &resCopy, logger) - if err != nil { - return nil, nil, err + if len(md) > 0 { + return &md } - - return u, res.generatedSecrets, nil + return nil } -func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - res, err := c.convertMonitorSpec(pm, logger, cache, true) +// convertToMonitoringResource is a parameterized helper that converts a PodMonitor to either a PodMonitoring or ClusterPodMonitoring resource. +func (c *PodMonitorConverter) convertToMonitoringResource( + pm *pomonitoringv1.PodMonitor, + logger *slog.Logger, + cache *ResourceCache, + isCluster bool, +) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + res, err := c.convertMonitorSpec(pm, logger, cache, isCluster) if err != nil { return nil, nil, err } - var filteredMetadata *[]string - if res.metadata != nil { - union := unionMetadata(*res.metadata, clusterMetadataDefaults) - filteredMetadata = &union - } - resCopy := *res - resCopy.metadata = filteredMetadata + resCopy.metadata = filterMetadata(res.metadata, isCluster, logger) + resCopy.metadata = resolveAttachMetadata(pm.Spec.AttachMetadata, resCopy.metadata, isCluster) - u, err := buildClusterPodMonitoring(pm.ObjectMeta, &resCopy, logger) + var u *unstructured.Unstructured + if isCluster { + u, err = buildClusterPodMonitoring(pm.ObjectMeta, &resCopy, logger) + } else { + u, err = buildPodMonitoring(pm.ObjectMeta, pm.Namespace, &resCopy, logger) + } if err != nil { return nil, nil, err } @@ -306,6 +303,14 @@ func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.P return u, res.generatedSecrets, nil } +func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + return c.convertToMonitoringResource(pm, logger, cache, false) +} + +func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + return c.convertToMonitoringResource(pm, logger, cache, true) +} + func unionMetadata(extracted []string, defaults []string) []string { unique := make(map[string]bool) for _, m := range defaults { From b81d029e48f2723d3fa193333ca865d7d0caf54d Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Fri, 31 Jul 2026 19:53:33 +0000 Subject: [PATCH 10/16] refactor: consolidate filterMetadata logic --- pkg/migrate/helpers.go | 13 +++++------ pkg/migrate/helpers_test.go | 17 +++++++------- pkg/migrate/podmonitor.go | 45 ++++--------------------------------- 3 files changed, 19 insertions(+), 56 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 33000bb8d5..3293b106a5 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -1164,16 +1164,15 @@ func validateScrapeProtocols(protocols []pomonitoringv1.ScrapeProtocol, logger * func resolveAttachMetadata(attachMetadata *pomonitoringv1.AttachMetadata, baseMetadata *[]string, isCluster bool) *[]string { if attachMetadata != nil && attachMetadata.Node != nil && *attachMetadata.Node { if baseMetadata == nil { + defaults := namespacedMetadataDefaults if isCluster { - union := unionMetadata([]string{labelNode}, clusterMetadataDefaults) - return &union + defaults = clusterMetadataDefaults } - return &[]string{labelNode} - } - if !slices.Contains(*baseMetadata, labelNode) { - metadataCopy := append(slices.Clone(*baseMetadata), labelNode) - return &metadataCopy + union := unionMetadata([]string{labelNode}, defaults) + return &union } + union := unionMetadata([]string{labelNode}, *baseMetadata) + return &union } return baseMetadata } diff --git a/pkg/migrate/helpers_test.go b/pkg/migrate/helpers_test.go index c9e9353010..98d275e6f5 100644 --- a/pkg/migrate/helpers_test.go +++ b/pkg/migrate/helpers_test.go @@ -22,6 +22,7 @@ import ( monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1" "github.com/google/go-cmp/cmp" pomonitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "github.com/prometheus/prometheus/google/export" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -827,9 +828,9 @@ func TestResolveAttachMetadata(t *testing.T) { { name: "nil attachMetadata returns base", attachMetadata: nil, - base: &[]string{"pod"}, + base: &[]string{labelPod}, isCluster: false, - expected: &[]string{"pod"}, + expected: &[]string{labelPod}, }, { name: "attachMetadata node false returns base", @@ -843,28 +844,28 @@ func TestResolveAttachMetadata(t *testing.T) { attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, base: nil, isCluster: false, - expected: &[]string{"node"}, + expected: &[]string{labelContainer, labelNode, labelPod, labelTopLevelControllerName, labelTopLevelControllerType}, }, { name: "cluster monitor with nil base returns node plus cluster defaults", attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, base: nil, isCluster: true, - expected: &[]string{"container", "namespace", "node", "pod", "top_level_controller_name", "top_level_controller_type"}, + expected: &[]string{labelContainer, export.KeyNamespace, labelNode, labelPod, labelTopLevelControllerName, labelTopLevelControllerType}, }, { name: "namespaced monitor with existing base appends node", attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, - base: &[]string{"pod"}, + base: &[]string{labelPod}, isCluster: false, - expected: &[]string{"pod", "node"}, + expected: &[]string{labelNode, labelPod}, }, { name: "namespaced monitor with node already present does not duplicate", attachMetadata: &pomonitoringv1.AttachMetadata{Node: ptrTo(true)}, - base: &[]string{"pod", "node"}, + base: &[]string{labelNode, labelPod}, isCluster: false, - expected: &[]string{"pod", "node"}, + expected: &[]string{labelNode, labelPod}, }, } diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index 0c02885aa7..ae862d5d0d 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -195,40 +195,7 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, resolveScrapeClass(pm.Spec.ScrapeClassName, logger) validateScrapeProtocols(pm.Spec.ScrapeProtocols, logger) - var filteredMetadata *[]string - if isCluster { - if rules.ResourceCombined.Metadata != nil { - union := unionMetadata(*rules.ResourceCombined.Metadata, clusterMetadataDefaults) - filteredMetadata = &union - } - } else { - if rules.ResourceCombined.Metadata != nil { - union := unionMetadata(*rules.ResourceCombined.Metadata, namespacedMetadataDefaults) - var md []string - for _, m := range union { - if m != export.KeyNamespace { - md = append(md, m) - } else { - logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The metadata entry has been omitted .") - } - } - if len(md) > 0 { - filteredMetadata = &md - } - } - } - - // In GMP, Metadata: nil on a PodMonitoring defaults to emitting namespaced defaults (container, pod, etc.). - // When setting Metadata explicitly for AttachMetadata.Node, we must merge namespacedMetadataDefaults so that default metadata is not dropped. - if pm.Spec.AttachMetadata != nil && pm.Spec.AttachMetadata.Node != nil && *pm.Spec.AttachMetadata.Node { - if filteredMetadata == nil { - union := unionMetadata([]string{labelNode}, namespacedMetadataDefaults) - filteredMetadata = &union - } else { - union := unionMetadata([]string{labelNode}, *filteredMetadata) - filteredMetadata = &union - } - } + filteredMetadata := filterMetadata(rules.ResourceCombined.Metadata, isCluster, logger) filteredMetadata = resolveAttachMetadata(pm.Spec.AttachMetadata, filteredMetadata, isCluster) var filterRunnings []*bool @@ -265,7 +232,7 @@ func filterMetadata(metadata *[]string, isCluster bool, logger *slog.Logger) *[] if m != export.KeyNamespace { md = append(md, m) } else { - logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The rule has been dropped.") + logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The metadata entry has been omitted.") } } if len(md) > 0 { @@ -286,15 +253,11 @@ func (c *PodMonitorConverter) convertToMonitoringResource( return nil, nil, err } - resCopy := *res - resCopy.metadata = filterMetadata(res.metadata, isCluster, logger) - resCopy.metadata = resolveAttachMetadata(pm.Spec.AttachMetadata, resCopy.metadata, isCluster) - var u *unstructured.Unstructured if isCluster { - u, err = buildClusterPodMonitoring(pm.ObjectMeta, &resCopy, logger) + u, err = buildClusterPodMonitoring(pm.ObjectMeta, res, logger) } else { - u, err = buildPodMonitoring(pm.ObjectMeta, pm.Namespace, &resCopy, logger) + u, err = buildPodMonitoring(pm.ObjectMeta, pm.Namespace, res, logger) } if err != nil { return nil, nil, err From b6973fe84fcbc9f15c99a2cf47989ca4ede38205 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Fri, 31 Jul 2026 20:56:35 +0000 Subject: [PATCH 11/16] fix: resolve namespace, nil, and typo --- pkg/migrate/helpers.go | 5 +- pkg/migrate/podmonitor.go | 17 +++--- pkg/migrate/podmonitor_test.go | 96 ++++++++++++++++++++++++++++++++-- 3 files changed, 103 insertions(+), 15 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 3293b106a5..a3abc94ebc 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -1256,7 +1256,7 @@ func warnUnsupportedEndpointFields(logger *slog.Logger, followRedirects *bool, e logger.Warn(fmt.Sprintf("endpoint [%d]: field 'honorTimestamps: true' is unsupported and dropped. GMP always uses the scrape ingestion timestamp. Target metric timestamps will be ignored.", i)) } if trackTimestampsStaleness != nil { - logger.Warn(fmt.Sprintf("endpoint [%d]: fField 'trackTimestampsStaleness' is unsupported in GMP and has been dropped.", i)) + logger.Warn(fmt.Sprintf("endpoint [%d]: field 'trackTimestampsStaleness' is unsupported in GMP and has been dropped.", i)) } } @@ -1275,5 +1275,8 @@ func combineAndConvertRelabelings(logger *slog.Logger, promoted []monitoringv1.R allRules = append(allRules, rules...) } + if len(allRules) == 0 { + return nil + } return allRules } diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index ae862d5d0d..a49502edf9 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -83,18 +83,17 @@ func (c *PodMonitorConverter) Convert(_ context.Context, logger *slog.Logger, un ) } - baseU, generatedSecrets, err := c.convertToPodMonitoring(&podMonitor, logger, cache) - if err != nil { - return nil, err - } - var outputs []*unstructured.Unstructured for _, ns := range targetNamespaces { - uClone := baseU.DeepCopy() - uClone.SetNamespace(ns) - outputs = append(outputs, uClone) + pmCopy := podMonitor.DeepCopy() + pmCopy.Namespace = ns + u, generatedSecrets, err := c.convertToPodMonitoring(pmCopy, logger, cache) + if err != nil { + return nil, err + } + outputs = append(outputs, u) + outputs = append(outputs, generatedSecrets...) } - outputs = append(outputs, generatedSecrets...) return outputs, nil } diff --git a/pkg/migrate/podmonitor_test.go b/pkg/migrate/podmonitor_test.go index 44d78a4b28..564a43d315 100644 --- a/pkg/migrate/podmonitor_test.go +++ b/pkg/migrate/podmonitor_test.go @@ -42,7 +42,7 @@ func TestPodMonitorConversion(t *testing.T) { dontWantWarnings []string }{ { - name: "Case A: Cluster-Scoped (Any Namespace)", + name: "Cluster-Scoped (Any Namespace)", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -83,7 +83,7 @@ func TestPodMonitorConversion(t *testing.T) { }, }, { - name: "Case B: Multi-Namespace Split", + name: "Multi-Namespace Split", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -140,7 +140,7 @@ func TestPodMonitorConversion(t *testing.T) { }, }, { - name: "Case B.2: Namespace Deduplication & Trimming", + name: "Namespace Deduplication & Trimming", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -180,7 +180,93 @@ func TestPodMonitorConversion(t *testing.T) { }, }, { - name: "Case B.3: Broken Config", + name: "Multiple Target Namespaces sets SecretReferences to target namespace upon creation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + NamespaceSelector: pomonitoringv1.NamespaceSelector{ + MatchNames: []string{"ns-1", "ns-2"}, + }, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "secret-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "metrics", + BasicAuth: &pomonitoringv1.BasicAuth{ + Username: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "auth"}, Key: "user"}, + Password: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "auth"}, Key: "pass"}, + }, + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-monitor", + Namespace: "ns-1", + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "secret-app"}, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + BasicAuth: &monitoringv1.BasicAuth{ + Username: "", + Password: &monitoringv1.SecretSelector{Secret: &monitoringv1.SecretKeySelector{Name: "auth", Key: "pass", Namespace: "ns-1"}}, + }, + }, + }, + }, + }, + }, + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-monitor", + Namespace: "ns-2", + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "secret-app"}, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + BasicAuth: &monitoringv1.BasicAuth{ + Username: "", + Password: &monitoringv1.SecretSelector{Secret: &monitoringv1.SecretKeySelector{Name: "auth", Key: "pass", Namespace: "ns-2"}}, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "Broken Config", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -202,7 +288,7 @@ func TestPodMonitorConversion(t *testing.T) { wantErr: "namespaceSelector.matchNames contains only empty or invalid values", }, { - name: "Case C: Local Scoping (Omitted Selector)", + name: "Local Scoping (Omitted Selector)", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", From 92db8940250b30fe50c30c51f67c2e11a2401b73 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Tue, 4 Aug 2026 18:43:36 +0000 Subject: [PATCH 12/16] fix: remove metadata empty check --- pkg/migrate/podmonitor.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index a49502edf9..e0114dfa14 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -234,10 +234,7 @@ func filterMetadata(metadata *[]string, isCluster bool, logger *slog.Logger) *[] logger.Warn("Relabeling rule referencing namespace metadata is unsupported in namespaced PodMonitoring (it is only allowed in ClusterPodMonitoring). The metadata entry has been omitted.") } } - if len(md) > 0 { - return &md - } - return nil + return &md } // convertToMonitoringResource is a parameterized helper that converts a PodMonitor to either a PodMonitoring or ClusterPodMonitoring resource. From 9b4fc11b34524b1c5ea6ca0fc39cbd400257888a Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Wed, 5 Aug 2026 14:25:41 +0000 Subject: [PATCH 13/16] fix: set target namesapce correctly --- pkg/migrate/helpers.go | 27 ++++++++++++++----- pkg/migrate/helpers_test.go | 52 ++++++++++++++++++++++++++++++++++--- pkg/migrate/podmonitor.go | 28 ++++++++++---------- pkg/migrate/types.go | 12 --------- 4 files changed, 83 insertions(+), 36 deletions(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index a3abc94ebc..571efa2de0 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -154,13 +154,26 @@ func determineNamespaceScoping(nsSel pomonitoringv1.NamespaceSelector, defaultNS return []string{defaultNS}, false, nil } +// commonMonitorSpec holds common fields extracted from Prometheus Operator monitor specs for building GMP resources. +type commonMonitorSpec struct { + endpoints []monitoringv1.ScrapeEndpoint + mergedFromPod []monitoringv1.LabelMapping + mergedSelector metav1.LabelSelector + metadata *[]string + filterRunning *bool + limits *monitoringv1.ScrapeLimits + generatedSecrets []*unstructured.Unstructured +} + // conversionContext groups common parameters passed down to conversion helper functions. type conversionContext struct { logger *slog.Logger // cache provides access to dependent resources. cache *ResourceCache - // namespace is the source namespace of the primary resource. - namespace string + // sourceNamespace is the original namespace where inputs (Secrets/ConfigMaps) live in the cache. + sourceNamespace string + // targetNamespace is the destination namespace for generated resources. + targetNamespace string // generatedSecrets accumulates created Secrets when migrating ConfigMaps, keyed by Secret name. generatedSecrets map[string]*unstructured.Unstructured } @@ -380,7 +393,7 @@ func (c *conversionContext) extractResourceKey(kind, name, key string) (string, return "", fmt.Errorf("%s reference has an empty key for name %q", kindUpper, name) } - obj, ok := c.cache.Get(kind, c.namespace, name) + obj, ok := c.cache.Get(kind, c.sourceNamespace, name) if !ok { c.logger.Warn("Resource not found in cache. Cannot extract key. Hardcoding placeholder.", slog.String("referenced_kind", kind), @@ -469,7 +482,7 @@ func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigM } if _, exists := c.generatedSecrets[secretName]; !exists { - obj, ok := c.cache.Get(KindConfigMap, c.namespace, sel.Name) + obj, ok := c.cache.Get(KindConfigMap, c.sourceNamespace, sel.Name) if !ok { c.logger.Warn("TLS ConfigMap reference was not found in the inputs. Updated reference to GMP Secret, but you must manually convert your ConfigMap to a Secret with this name in GMP.", slog.String("configmap", sel.Name), @@ -483,7 +496,7 @@ func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigM newSecret.SetAPIVersion("v1") newSecret.SetKind(KindSecret) newSecret.SetName(secretName) - newSecret.SetNamespace(c.namespace) + newSecret.SetNamespace(c.targetNamespace) data, found, _ := unstructured.NestedMap(obj.Object, "data") if found { @@ -497,7 +510,7 @@ func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigM } } - secretRef := &monitoringv1.SecretKeySelector{Name: secretName, Key: secretKey, Namespace: c.namespace} + secretRef := &monitoringv1.SecretKeySelector{Name: secretName, Key: secretKey, Namespace: c.targetNamespace} return &monitoringv1.SecretSelector{Secret: secretRef}, nil } @@ -531,7 +544,7 @@ func (c *conversionContext) convertSecretSelector(sel *corev1.SecretKeySelector) c.logger.Warn("Secret reference had 'optional: true'. GMP does not support optional secrets. The reference is now mandatory.", slog.String("secret", sel.Name)) } - secretRef := &monitoringv1.SecretKeySelector{Name: sel.Name, Key: sel.Key, Namespace: c.namespace} + secretRef := &monitoringv1.SecretKeySelector{Name: sel.Name, Key: sel.Key, Namespace: c.targetNamespace} return &monitoringv1.SecretSelector{Secret: secretRef}, nil } diff --git a/pkg/migrate/helpers_test.go b/pkg/migrate/helpers_test.go index 98d275e6f5..1cd55a9bbd 100644 --- a/pkg/migrate/helpers_test.go +++ b/pkg/migrate/helpers_test.go @@ -31,9 +31,10 @@ import ( func newTestConversionContext() *conversionContext { return &conversionContext{ - logger: slog.New(slog.NewTextHandler(os.Stdout, nil)), - cache: NewResourceCache(), - namespace: "default", + logger: slog.New(slog.NewTextHandler(os.Stdout, nil)), + cache: NewResourceCache(), + sourceNamespace: "default", + targetNamespace: "default", } } @@ -916,3 +917,48 @@ func TestToStrictUnstructured(t *testing.T) { }) } } + +func TestDecoupledNamespaces(t *testing.T) { + ctx := newTestConversionContext() + ctx.sourceNamespace = "source-ns" + ctx.targetNamespace = "target-ns" + + // Verify that secret extraction reads from sourceNamespace. + if err := addSecretToCache(ctx.cache, "source-ns", "my-secret", "user", "admin", true); err != nil { + t.Fatalf("failed to add secret to cache: %v", err) + } + sel := corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret"}, + Key: "user", + } + val, err := ctx.extractSecretKey(sel) + if err != nil { + t.Fatalf("extractSecretKey() unexpected error: %v", err) + } + if val != "admin" { + t.Errorf("extractSecretKey() = %q, want %q", val, "admin") + } + + // Verify that ConfigMap to Secret conversion reads from sourceNamespace and generates Secret in targetNamespace. + if err := addConfigMapToCache(ctx.cache, "source-ns", "tls-cm", "ca.crt", "cert-data"); err != nil { + t.Fatalf("failed to add configmap to cache: %v", err) + } + cmSel := &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "tls-cm"}, + Key: "ca.crt", + } + secretSel, err := ctx.convertConfigMapToSecretSelector(cmSel) + if err != nil { + t.Fatalf("convertConfigMapToSecretSelector() unexpected error: %v", err) + } + if secretSel.Secret.Namespace != "target-ns" { + t.Errorf("expected selector namespace %q, got %q", "target-ns", secretSel.Secret.Namespace) + } + genSecrets := ctx.getGeneratedSecrets() + if len(genSecrets) != 1 { + t.Fatalf("expected 1 generated secret, got %d", len(genSecrets)) + } + if genSecrets[0].GetNamespace() != "target-ns" { + t.Errorf("expected generated secret namespace %q, got %q", "target-ns", genSecrets[0].GetNamespace()) + } +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index e0114dfa14..d6b36215ab 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -68,7 +68,7 @@ func (c *PodMonitorConverter) Convert(_ context.Context, logger *slog.Logger, un if isClusterScoped { logger.Info("namespaceSelector selects 'any: true'. Translated to 'ClusterPodMonitoring'") - u, generatedSecrets, err := c.convertToClusterPodMonitoring(&podMonitor, logger, cache) + u, generatedSecrets, err := c.convertToClusterPodMonitoring(&podMonitor, podMonitor.Namespace, logger, cache) if err != nil { return nil, err } @@ -85,9 +85,7 @@ func (c *PodMonitorConverter) Convert(_ context.Context, logger *slog.Logger, un var outputs []*unstructured.Unstructured for _, ns := range targetNamespaces { - pmCopy := podMonitor.DeepCopy() - pmCopy.Namespace = ns - u, generatedSecrets, err := c.convertToPodMonitoring(pmCopy, logger, cache) + u, generatedSecrets, err := c.convertToPodMonitoring(&podMonitor, ns, logger, cache) if err != nil { return nil, err } @@ -161,11 +159,12 @@ func (c *PodMonitorConverter) convertEndpoints( return gmpEndpoints, nil } -func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache, isCluster bool) (*commonMonitorSpec, error) { +func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, targetNamespace string, logger *slog.Logger, cache *ResourceCache, isCluster bool) (*commonMonitorSpec, error) { convCtx := &conversionContext{ - logger: logger, - cache: cache, - namespace: pm.Namespace, + logger: logger, + cache: cache, + sourceNamespace: pm.Namespace, + targetNamespace: targetNamespace, } rules, err := extractPreScrapeRelabelings(logger, pm.Spec.PodMetricsEndpoints) if err != nil { @@ -240,11 +239,12 @@ func filterMetadata(metadata *[]string, isCluster bool, logger *slog.Logger) *[] // convertToMonitoringResource is a parameterized helper that converts a PodMonitor to either a PodMonitoring or ClusterPodMonitoring resource. func (c *PodMonitorConverter) convertToMonitoringResource( pm *pomonitoringv1.PodMonitor, + targetNamespace string, logger *slog.Logger, cache *ResourceCache, isCluster bool, ) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - res, err := c.convertMonitorSpec(pm, logger, cache, isCluster) + res, err := c.convertMonitorSpec(pm, targetNamespace, logger, cache, isCluster) if err != nil { return nil, nil, err } @@ -253,7 +253,7 @@ func (c *PodMonitorConverter) convertToMonitoringResource( if isCluster { u, err = buildClusterPodMonitoring(pm.ObjectMeta, res, logger) } else { - u, err = buildPodMonitoring(pm.ObjectMeta, pm.Namespace, res, logger) + u, err = buildPodMonitoring(pm.ObjectMeta, targetNamespace, res, logger) } if err != nil { return nil, nil, err @@ -262,12 +262,12 @@ func (c *PodMonitorConverter) convertToMonitoringResource( return u, res.generatedSecrets, nil } -func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - return c.convertToMonitoringResource(pm, logger, cache, false) +func (c *PodMonitorConverter) convertToPodMonitoring(pm *pomonitoringv1.PodMonitor, targetNamespace string, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + return c.convertToMonitoringResource(pm, targetNamespace, logger, cache, false) } -func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { - return c.convertToMonitoringResource(pm, logger, cache, true) +func (c *PodMonitorConverter) convertToClusterPodMonitoring(pm *pomonitoringv1.PodMonitor, targetNamespace string, logger *slog.Logger, cache *ResourceCache) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { + return c.convertToMonitoringResource(pm, targetNamespace, logger, cache, true) } func unionMetadata(extracted []string, defaults []string) []string { diff --git a/pkg/migrate/types.go b/pkg/migrate/types.go index aff2bd7f56..8fab955448 100644 --- a/pkg/migrate/types.go +++ b/pkg/migrate/types.go @@ -20,8 +20,6 @@ import ( "fmt" "log/slog" - monitoringv1 "github.com/GoogleCloudPlatform/prometheus-engine/pkg/operator/apis/monitoring/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -39,16 +37,6 @@ const ( KindSecret = "Secret" ) -type commonMonitorSpec struct { - endpoints []monitoringv1.ScrapeEndpoint - mergedFromPod []monitoringv1.LabelMapping - mergedSelector metav1.LabelSelector - metadata *[]string - filterRunning *bool - limits *monitoringv1.ScrapeLimits - generatedSecrets []*unstructured.Unstructured -} - // ResourceConverter defines the interface for converting a specific Prometheus Operator resource kind. type ResourceConverter interface { // ImportKey returns the Kind of the resource this converter handles (e.g., "PodMonitor"). From 1852ac8754bcb3489c2236df4bdd683b258315f5 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Wed, 5 Aug 2026 18:53:51 +0000 Subject: [PATCH 14/16] fix: remove expectErr on ToStrictUnstructured test --- pkg/migrate/helpers_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/migrate/helpers_test.go b/pkg/migrate/helpers_test.go index 1cd55a9bbd..efca7e1f48 100644 --- a/pkg/migrate/helpers_test.go +++ b/pkg/migrate/helpers_test.go @@ -882,9 +882,8 @@ func TestResolveAttachMetadata(t *testing.T) { func TestToStrictUnstructured(t *testing.T) { tests := []struct { - name string - obj any - expectErr bool + name string + obj any }{ { name: "scrape limits with uint64 converts to int64", @@ -892,18 +891,14 @@ func TestToStrictUnstructured(t *testing.T) { Samples: 5000, Labels: 100, }, - expectErr: false, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { u, err := toStrictUnstructured(tc.obj) - if (err != nil) != tc.expectErr { - t.Fatalf("toStrictUnstructured() error = %v, expectErr = %v", err, tc.expectErr) - } - if tc.expectErr { - return + if err != nil { + t.Fatalf("unexpected error: %v", err) } // Verify uint64 fields are converted to int64. if val, ok := u["samples"]; ok { From 450e1aeba66a9f3fe1ca716247fb4c09622525b1 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Wed, 5 Aug 2026 18:55:03 +0000 Subject: [PATCH 15/16] fix: handle bearerToken key empty --- pkg/migrate/helpers.go | 2 +- pkg/migrate/podmonitor_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 571efa2de0..35cf3d1359 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -704,7 +704,7 @@ func (c *conversionContext) applyAuthAndTLS( } // Handle deprecated BearerTokenSecret -> Authorization. - if bearerTokenSecret.Name != "" { // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. + if bearerTokenSecret.Name != "" || bearerTokenSecret.Key != "" { // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. if gmpEp.Authorization != nil { c.logger.Warn("Endpoint has both 'bearerTokenSecret' and 'authorization' defined. Dropping 'bearerTokenSecret'.") } else { diff --git a/pkg/migrate/podmonitor_test.go b/pkg/migrate/podmonitor_test.go index 564a43d315..42e038ed5d 100644 --- a/pkg/migrate/podmonitor_test.go +++ b/pkg/migrate/podmonitor_test.go @@ -1633,6 +1633,31 @@ func TestPodMonitorConversion(t *testing.T) { }, wantErr: "conflicting keep rules for label \"env\": cannot require both \"production\" and \"staging\" simultaneously", }, + { + name: "BearerTokenSecret with empty Name returns validation error", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bearer-token-secret-err", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "frontend"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "metrics", + BearerTokenSecret: corev1.SecretKeySelector{Key: "token"}, + }, + }, + }, + }, + wantErr: "bearerTokenSecret: secret reference has an empty name for key \"token\"", + }, { name: "Pre-Scrape Relabelings: drop action rule on pod label falls through to metricRelabelings", input: &pomonitoringv1.PodMonitor{ From 21cbeb7b46528593b898ef25eae8d97a26835936 Mon Sep 17 00:00:00 2001 From: Karthik Unnikrishnan Date: Wed, 5 Aug 2026 20:26:23 +0000 Subject: [PATCH 16/16] fix: add TODO for filterRunning optimization --- pkg/migrate/helpers.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index 35cf3d1359..5506e9d113 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -1191,6 +1191,7 @@ func resolveAttachMetadata(attachMetadata *pomonitoringv1.AttachMetadata, baseMe } // resolveFilterRunning evaluates filterRunning settings across endpoints and resolves them to a single resource-level setting. +// TODO: Potential better optimization would be to split into separate PodMonitorings based on filterRunning value. func resolveFilterRunning(filterRunnings []*bool, logger *slog.Logger, isCluster bool) *bool { var hasFalse, hasTrue bool for _, fr := range filterRunnings {