diff --git a/cmd/gmp-migrate/main.go b/cmd/gmp-migrate/main.go index 72ae25bd1c..80e24df698 100644 --- a/cmd/gmp-migrate/main.go +++ b/cmd/gmp-migrate/main.go @@ -49,6 +49,10 @@ func main() { flag.Var(&inputFiles, "file", "Input source (YAML file, directory, or '-' for stdin) (Required)") flag.Var(&inputFiles, "f", "Input source (YAML file, directory, or '-' for stdin) (Required)") + var emitAll bool + flag.BoolVar(&emitAll, "all", false, "Emit all manifests, including best-effort draft configurations with TODO annotations") + flag.BoolVar(&emitAll, "a", false, "Emit all manifests, including best-effort draft configurations with TODO annotations") + flag.Usage = func() { fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0]) fmt.Fprint(os.Stderr, "Migrate Prometheus Operator configurations to Google Managed Prometheus (GMP).\n\n") @@ -87,19 +91,30 @@ func main() { // If any resource failed to convert in-memory, we print summary and abort. if report.FailedCount > 0 { - migrator.PrintSummary(report) // Still print the diagnostic summary to Stderr. + migrator.PrintSummary(report, emitAll) // Still print the diagnostic summary to Stderr. slog.Error("Migration aborted: resources failed conversion. Zero manifests were written to Stdout.", slog.Int("failures", report.FailedCount), ) os.Exit(1) } + // Select outputs based on --all flag. + outputsToWrite := report.ReadyOutputs + if emitAll { + outputsToWrite = report.Outputs + } + // Write the converted GMP manifests using the migrator's Stdout stream. - if err := migrator.WriteOutputs(report.Outputs); err != nil { + if err := migrator.WriteOutputs(outputsToWrite); err != nil { slog.Error("Failed to write outputs", slog.Any("error", err)) os.Exit(1) } // Print the successful complete summary to Stderr. - migrator.PrintSummary(report) + migrator.PrintSummary(report, emitAll) + + // If any resource required action items or failed, exit with 1. + if report.ActionItemsCount > 0 || report.FailedCount > 0 { + os.Exit(1) + } } diff --git a/pkg/migrate/helpers.go b/pkg/migrate/helpers.go index eb9334e6c0..5c7ff91f2d 100644 --- a/pkg/migrate/helpers.go +++ b/pkg/migrate/helpers.go @@ -20,6 +20,7 @@ import ( "fmt" "hash/fnv" "log/slog" + "net/url" "slices" "strings" @@ -46,6 +47,10 @@ const ( labelAddress = "__address__" ) +const ( + AnnotationTodoPrefix = "gmp.googleapis.com/todo-" +) + // Constants representing the supported ScrapeProtocol enum values defined in upstream Prometheus Operator. const ( scrapeProtocolOpenMetricsText100 = pomonitoringv1.ScrapeProtocol("OpenMetricsText1.0.0") @@ -95,6 +100,7 @@ type preScrapeRelabelingResult struct { MatchLabels map[string]string MatchExpressions []metav1.LabelSelectorRequirement PromotedRules []monitoringv1.RelabelingRule + Todos []todoItem } // ExtractedPreScrapeRules holds all translated rules, separated by where they belong in GMP. @@ -121,12 +127,34 @@ func CopyObjectMeta(src metav1.ObjectMeta, targetNamespace string, logger *slog. } if len(src.Labels) > 0 || len(src.Annotations) > 0 { - logger.Warn("Stripped all metadata labels and annotations. Reconfigure them manually if needed") + logger.Info("Stripped all metadata labels and annotations. Reconfigure them manually if needed") } return dst } +// AddMigrationTodo appends a sequential TODO annotation to the unstructured resource. +func AddMigrationTodo(u *unstructured.Unstructured, category, reason, action string) { + if u == nil || u.Object == nil { + return + } + annotations := u.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + + todoNumber := 1 + for { + key := fmt.Sprintf("%s%d", AnnotationTodoPrefix, todoNumber) + if _, exists := annotations[key]; !exists { + annotations[key] = fmt.Sprintf("[%s] %s ACTION: %s", category, reason, action) + break + } + todoNumber++ + } + u.SetAnnotations(annotations) +} + // parseAndCleanNamespaces trims whitespace, filters out empty strings, and deduplicates namespaces. func parseAndCleanNamespaces(namespaces []string) []string { unique := make(map[string]bool) @@ -156,6 +184,13 @@ func determineNamespaceScoping(nsSel pomonitoringv1.NamespaceSelector, defaultNS return []string{defaultNS}, false, nil } +// todoItem represents an actionable TODO annotation to attach to generated resources. +type todoItem struct { + category string + reason string + action string +} + // commonMonitorSpec holds common fields extracted from Prometheus Operator monitor specs for building GMP resources. type commonMonitorSpec struct { endpoints []monitoringv1.ScrapeEndpoint @@ -165,6 +200,7 @@ type commonMonitorSpec struct { filterRunning *bool limits *monitoringv1.ScrapeLimits generatedSecrets []*unstructured.Unstructured + todos []todoItem } // conversionContext groups common parameters passed down to conversion helper functions. @@ -180,6 +216,7 @@ type conversionContext struct { generatedSecrets map[string]*unstructured.Unstructured // isClusterScoped indicates if the target resource is cluster-scoped (ClusterPodMonitoring). isClusterScoped bool + todos []todoItem } // getGeneratedSecrets returns the generated secrets accumulated in the context as a slice. @@ -224,7 +261,15 @@ func convertPreScrapeRelabelings(logger *slog.Logger, configs []pomonitoringv1.R action = relabel.Replace } - if shouldSkipRelabelConfig(logger, config, action) { + if skip, scopeExpanded, details := shouldSkipRelabelConfig(logger, config, action); skip { + if scopeExpanded { + logger.Warn("Scraping scope expanded: targets previously excluded by this rule will now be scraped. Adjust pod selectors to compensate.") + res.Todos = append(res.Todos, todoItem{ + category: "WARNING", + reason: fmt.Sprintf("Dropped target filtering rule (%s).", details), + action: "Add equivalent pod label selector in 'spec.selector.matchLabels'.", + }) + } continue } @@ -303,6 +348,7 @@ func extractPreScrapeRelabelings(logger *slog.Logger, endpointsRelabelConfigs [] if r.Metadata != nil { rawMetadata = append(rawMetadata, *r.Metadata...) } + combined.Todos = append(combined.Todos, r.Todos...) } epResults = append(epResults, r) } @@ -327,9 +373,9 @@ func extractPreScrapeRelabelings(logger *slog.Logger, endpointsRelabelConfigs [] } // mergeLabelSelector combines base selector requirements with extracted pre-scrape filtering rules. -func mergeLabelSelector(base metav1.LabelSelector, extraLabels map[string]string, extraExprs []metav1.LabelSelectorRequirement) (metav1.LabelSelector, error) { +func (c *conversionContext) mergeLabelSelector(base metav1.LabelSelector, extraLabels map[string]string, extraExprs []metav1.LabelSelectorRequirement) metav1.LabelSelector { if len(extraLabels) == 0 && len(extraExprs) == 0 { - return *base.DeepCopy(), nil + return *base.DeepCopy() } res := base.DeepCopy() if len(extraLabels) > 0 && res.MatchLabels == nil { @@ -337,12 +383,17 @@ func mergeLabelSelector(base metav1.LabelSelector, extraLabels map[string]string } for k, v := range extraLabels { if existing, exists := res.MatchLabels[k]; exists && existing != v { - return metav1.LabelSelector{}, fmt.Errorf("selector conflict: label %q has conflicting values %q (base selector) and %q (relabeling rule)", k, existing, v) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Selector conflict: label %q has conflicting values %q (from base selector) and %q (from relabeling rule).", k, existing, v), + action: fmt.Sprintf("Reconcile 'spec.selector.matchLabels' for label %q with the intended target pods.", k), + }) + continue } res.MatchLabels[k] = v } res.MatchExpressions = append(res.MatchExpressions, extraExprs...) - return *res, nil + return *res } // mergeFromPod merges target label mappings and deduplicates by target label name. @@ -383,29 +434,43 @@ func mergeFromPod(logger *slog.Logger, base []monitoringv1.LabelMapping, extra [ } // extractResourceKey is a consolidated helper that fetches a key from a ConfigMap or Secret. -// It returns an error if the reference is malformed, missing, or corrupt. -func (c *conversionContext) extractResourceKey(kind, name, key string) (string, error) { +func (c *conversionContext) extractResourceKey(kind, name, key string) string { kindUpper := strings.ToUpper(kind) if name == "" && key == "" { - return "", nil + return "" } if name == "" { - return "", fmt.Errorf("%s reference has an empty name for key %q", kindUpper, key) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced %s has an empty name for key %q.", kindUpper, key), + action: fmt.Sprintf("Specify a valid %s name in the configuration.", kindUpper), + }) + return fmt.Sprintf("TODO_SET_%s_FROM_%s_EMPTY_NAME", strings.ToUpper(key), kindUpper) } if key == "" { - return "", fmt.Errorf("%s reference has an empty key for name %q", kindUpper, name) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced %s %q has an empty key.", kindUpper, name), + action: fmt.Sprintf("Specify a valid key in %s %q.", kindUpper, name), + }) + return fmt.Sprintf("TODO_SET_EMPTY_KEY_FROM_%s_%s", kindUpper, strings.ToUpper(name)) } obj, ok := c.cache.Get(kind, c.sourceNamespace, name) if !ok { - return "", fmt.Errorf("%s %q for key %q not found in namespace %q", kind, name, key, c.sourceNamespace) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced %s %q for key %q was not found in migration inputs.", kindUpper, name, key), + action: fmt.Sprintf("Verify %s %q exists in namespace %q or provide it in migration inputs.", kindUpper, name, c.sourceNamespace), + }) + return fmt.Sprintf("TODO_SET_%s_FROM_%s_%s", strings.ToUpper(key), kindUpper, strings.ToUpper(name)) } // Secrets support unencoded stringData. if kind == KindSecret { val, found, _ := unstructured.NestedString(obj.Object, "stringData", key) if found { - return val, nil + return val } } @@ -415,11 +480,16 @@ func (c *conversionContext) extractResourceKey(kind, name, key string) (string, if kind == KindSecret { decoded, err := base64.StdEncoding.DecodeString(strings.TrimSpace(val)) if err != nil { - return "", fmt.Errorf("failed to decode base64 data for key %q in secret %q: %w", key, name, err) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Failed to base64-decode key %q in Secret %q.", key, name), + action: fmt.Sprintf("Ensure Secret %q contains valid base64 data (or use stringData).", name), + }) + return fmt.Sprintf("TODO_CORRUPT_SECRET_DATA_%s", strings.ToUpper(key)) } - return string(decoded), nil + return string(decoded) } - return val, nil + return val } // ConfigMaps can store base64 binaryData. @@ -428,48 +498,67 @@ func (c *conversionContext) extractResourceKey(kind, name, key string) (string, if found { decoded, err := base64.StdEncoding.DecodeString(strings.TrimSpace(val)) if err != nil { - return "", fmt.Errorf("failed to decode base64 binaryData for key %q in configmap %q: %w", key, name, err) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Failed to base64-decode key %q in ConfigMap %q.", key, name), + action: fmt.Sprintf("Ensure ConfigMap %q contains valid base64 binaryData.", name), + }) + return fmt.Sprintf("TODO_CORRUPT_CONFIGMAP_DATA_%s", strings.ToUpper(key)) } - return string(decoded), nil + return string(decoded) } } - return "", fmt.Errorf("key %q not found in %s %q", key, kindUpper, name) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Key %q was not found in referenced %s %q.", key, kindUpper, name), + action: fmt.Sprintf("Add the %q key to %s %q.", key, kindUpper, name), + }) + return fmt.Sprintf("TODO_MISSING_KEY_%s_IN_%s_%s", strings.ToUpper(key), kindUpper, strings.ToUpper(name)) } // extractSecretKey extracts a string value from a Secret. -// It returns an error if the reference or data is malformed, or a placeholder if the resource is not found. -func (c *conversionContext) extractSecretKey(sel corev1.SecretKeySelector) (string, error) { +func (c *conversionContext) extractSecretKey(sel corev1.SecretKeySelector) string { if sel.Name == "" && sel.Key == "" { - return "", nil + return "" } return c.extractResourceKey(KindSecret, sel.Name, sel.Key) } // extractConfigMapKey extracts a string value from a ConfigMap. -// It returns an error if the reference or data is malformed, or a placeholder if the resource is not found. -func (c *conversionContext) extractConfigMapKey(sel corev1.ConfigMapKeySelector) (string, error) { +func (c *conversionContext) extractConfigMapKey(sel corev1.ConfigMapKeySelector) string { if sel.Name == "" && sel.Key == "" { - return "", nil + return "" } return c.extractResourceKey(KindConfigMap, sel.Name, sel.Key) } // convertConfigMapToSecretSelector translates a ConfigMapKeySelector to a SecretSelector. -// It returns an error if the reference is malformed. -func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigMapKeySelector) (*monitoringv1.SecretSelector, error) { +func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigMapKeySelector) *monitoringv1.SecretSelector { if sel == nil || (sel.Name == "" && sel.Key == "") { - return nil, nil + return nil } - if sel.Name == "" { - return nil, fmt.Errorf("configmap reference has an empty name for key %q", sel.Key) + name := sel.Name + key := sel.Key + if name == "" { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced ConfigMap has an empty name for key %q.", key), + action: "Specify a valid ConfigMap name in the configuration.", + }) + name = "TODO_SET_CONFIGMAP_NAME" } - if sel.Key == "" { - return nil, fmt.Errorf("configmap reference has an empty key for name %q", sel.Name) + if key == "" { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced ConfigMap %q has an empty key.", name), + action: fmt.Sprintf("Specify a valid key in ConfigMap %q.", name), + }) + key = "TODO_SET_CONFIGMAP_KEY" } - secretName := "secret-" + sel.Name - secretKey := sel.Key + secretName := "secret-" + name + secretKey := key if sel.Optional != nil && *sel.Optional { c.logger.Warn("ConfigMap reference had 'optional: true'. GMP does not support optional secrets. The reference is now mandatory.", @@ -514,12 +603,11 @@ func (c *conversionContext) convertConfigMapToSecretSelector(sel *corev1.ConfigM ns = c.targetNamespace } secretRef := &monitoringv1.SecretKeySelector{Name: secretName, Key: secretKey, Namespace: ns} - return &monitoringv1.SecretSelector{Secret: secretRef}, nil + return &monitoringv1.SecretSelector{Secret: secretRef} } // convertSecretOrConfigMapToSecretSelector translates a SecretOrConfigMap to a SecretSelector. -// It returns an error if the selected configuration reference is malformed. -func (c *conversionContext) convertSecretOrConfigMapToSecretSelector(sel pomonitoringv1.SecretOrConfigMap) (*monitoringv1.SecretSelector, error) { +func (c *conversionContext) convertSecretOrConfigMapToSecretSelector(sel pomonitoringv1.SecretOrConfigMap) *monitoringv1.SecretSelector { if sel.Secret != nil { return c.convertSecretSelector(sel.Secret) } @@ -528,58 +616,61 @@ func (c *conversionContext) convertSecretOrConfigMapToSecretSelector(sel pomonit return c.convertConfigMapToSecretSelector(sel.ConfigMap) } - return nil, nil + return nil } // convertSecretSelector translates a SecretKeySelector to a SecretSelector. -// It returns an error if the reference is malformed. -func (c *conversionContext) convertSecretSelector(sel *corev1.SecretKeySelector) (*monitoringv1.SecretSelector, error) { +func (c *conversionContext) convertSecretSelector(sel *corev1.SecretKeySelector) *monitoringv1.SecretSelector { if sel == nil || (sel.Name == "" && sel.Key == "") { - return nil, nil + return nil } - if sel.Name == "" { - return nil, fmt.Errorf("secret reference has an empty name for key %q", sel.Key) + name := sel.Name + key := sel.Key + if name == "" { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced Secret has an empty name for key %q.", key), + action: "Specify a valid Secret name in the configuration.", + }) + name = "TODO_SET_SECRET_NAME" } - if sel.Key == "" { - return nil, fmt.Errorf("secret reference has an empty key for name %q", sel.Name) + if key == "" { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Referenced Secret %q has an empty key.", name), + action: fmt.Sprintf("Specify a valid key in Secret %q.", name), + }) + key = "TODO_SET_SECRET_KEY" } if sel.Optional != nil && *sel.Optional { c.logger.Warn("Secret reference had 'optional: true'. GMP does not support optional secrets. The reference is now mandatory.", - slog.String("secret", sel.Name)) + slog.String("secret", name)) } var ns string if c.isClusterScoped { ns = c.targetNamespace } - secretRef := &monitoringv1.SecretKeySelector{Name: sel.Name, Key: sel.Key, Namespace: ns} - return &monitoringv1.SecretSelector{Secret: secretRef}, nil + secretRef := &monitoringv1.SecretKeySelector{Name: name, Key: key, Namespace: ns} + return &monitoringv1.SecretSelector{Secret: secretRef} } // convertBasicAuth maps PO BasicAuth to GMP BasicAuth, extracting the username string. -// It returns an error if either the username or password secret reference is malformed or invalid. -func (c *conversionContext) convertBasicAuth(ba *pomonitoringv1.BasicAuth) (*monitoringv1.BasicAuth, error) { +func (c *conversionContext) convertBasicAuth(ba *pomonitoringv1.BasicAuth) *monitoringv1.BasicAuth { if ba == nil { - return nil, nil - } - username, err := c.extractSecretKey(ba.Username) - if err != nil { - return nil, err - } - password, err := c.convertSecretSelector(&ba.Password) - if err != nil { - return nil, err + return nil } + username := c.extractSecretKey(ba.Username) + password := c.convertSecretSelector(&ba.Password) return &monitoringv1.BasicAuth{ Username: username, Password: password, - }, nil + } } // convertSafeTLSConfig maps PO SafeTLSConfig to GMP TLS, wrapping ConfigMaps into Secrets. -// It returns an error if any referenced certificate secret or configmap is malformed. -func (c *conversionContext) convertSafeTLSConfig(tls *pomonitoringv1.SafeTLSConfig) (*monitoringv1.TLS, error) { +func (c *conversionContext) convertSafeTLSConfig(tls *pomonitoringv1.SafeTLSConfig) *monitoringv1.TLS { if tls == nil { - return nil, nil + return nil } gmpTLS := &monitoringv1.TLS{} if tls.InsecureSkipVerify != nil { @@ -589,82 +680,70 @@ func (c *conversionContext) convertSafeTLSConfig(tls *pomonitoringv1.SafeTLSConf gmpTLS.ServerName = *tls.ServerName } if tls.CA.Secret != nil || tls.CA.ConfigMap != nil { - ca, err := c.convertSecretOrConfigMapToSecretSelector(tls.CA) - if err != nil { - return nil, err - } - gmpTLS.CA = ca + gmpTLS.CA = c.convertSecretOrConfigMapToSecretSelector(tls.CA) } if tls.Cert.Secret != nil || tls.Cert.ConfigMap != nil { - cert, err := c.convertSecretOrConfigMapToSecretSelector(tls.Cert) - if err != nil { - return nil, err - } - gmpTLS.Cert = cert + gmpTLS.Cert = c.convertSecretOrConfigMapToSecretSelector(tls.Cert) } if tls.KeySecret != nil { - key, err := c.convertSecretSelector(tls.KeySecret) - if err != nil { - return nil, err - } - gmpTLS.Key = key + gmpTLS.Key = c.convertSecretSelector(tls.KeySecret) } - return gmpTLS, nil + return gmpTLS } // convertOAuth2 maps PO OAuth2 to GMP OAuth2, extracting the clientID string. -// It returns an error if any secret or configmap reference is malformed or invalid. -func (c *conversionContext) convertOAuth2(oa *pomonitoringv1.OAuth2) (*monitoringv1.OAuth2, error) { +func (c *conversionContext) convertOAuth2(oa *pomonitoringv1.OAuth2) *monitoringv1.OAuth2 { if oa == nil { - return nil, nil + return nil } clientID := "" - var err error if oa.ClientID.Secret != nil { - clientID, err = c.extractSecretKey(*oa.ClientID.Secret) + clientID = c.extractSecretKey(*oa.ClientID.Secret) } else if oa.ClientID.ConfigMap != nil { - clientID, err = c.extractConfigMapKey(*oa.ClientID.ConfigMap) + clientID = c.extractConfigMapKey(*oa.ClientID.ConfigMap) } else { - return nil, errors.New("OAuth2 clientID must be defined as either Secret or ConfigMap") - } - if err != nil { - return nil, err + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: "OAuth2 clientID must be defined as either Secret or ConfigMap.", + action: "Specify a valid Secret or ConfigMap reference for 'clientID'.", + }) + clientID = "TODO_SET_OAUTH2_CLIENT_ID" } - clientSecret, err := c.convertSecretSelector(&oa.ClientSecret) - if err != nil { - return nil, err + clientSecret := c.convertSecretSelector(&oa.ClientSecret) + + tokenURL := oa.TokenURL + if tokenURL == "" { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: "OAuth2 tokenURL is empty.", + action: "Specify a valid token endpoint URL in 'spec.endpoints[].oauth2.tokenUrl'.", + }) + tokenURL = "TODO_SET_OAUTH2_TOKEN_URL" } return &monitoringv1.OAuth2{ ClientID: clientID, ClientSecret: clientSecret, - TokenURL: oa.TokenURL, + TokenURL: tokenURL, Scopes: oa.Scopes, EndpointParams: oa.EndpointParams, - }, nil + } } // convertAuthorization maps PO SafeAuthorization to GMP Auth. -// It returns an error if the credentials secret reference is malformed. -func (c *conversionContext) convertAuthorization(auth *pomonitoringv1.SafeAuthorization) (*monitoringv1.Auth, error) { +func (c *conversionContext) convertAuthorization(auth *pomonitoringv1.SafeAuthorization) *monitoringv1.Auth { if auth == nil { - return nil, nil + return nil } - var ( - credentials *monitoringv1.SecretSelector - err error - ) + var credentials *monitoringv1.SecretSelector if auth.Credentials != nil { - credentials, err = c.convertSecretSelector(auth.Credentials) - if err != nil { - return nil, err - } + credentials = c.convertSecretSelector(auth.Credentials) } return &monitoringv1.Auth{ Type: auth.Type, Credentials: credentials, - }, nil + } } // applyAuthAndTLS converts credentials and TLS settings for a generic endpoint. @@ -675,38 +754,18 @@ func (c *conversionContext) applyAuthAndTLS( tlsConfig *pomonitoringv1.SafeTLSConfig, 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 { - return fmt.Errorf("basicAuth: %w", err) - } - gmpEp.BasicAuth = ba + gmpEp.BasicAuth = c.convertBasicAuth(basicAuth) } if oAuth2 != nil { - oa, err := c.convertOAuth2(oAuth2) - if err != nil { - return fmt.Errorf("oAuth2: %w", err) - } - gmpEp.OAuth2 = oa + gmpEp.OAuth2 = c.convertOAuth2(oAuth2) } if tlsConfig != nil { - tls, err := c.convertSafeTLSConfig(tlsConfig) - if err != nil { - return fmt.Errorf("tlsConfig: %w", err) - } - gmpEp.TLS = tls + gmpEp.TLS = c.convertSafeTLSConfig(tlsConfig) } if authorization != nil { - auth, err := c.convertAuthorization(authorization) - if err != nil { - return fmt.Errorf("authorization: %w", err) - } - gmpEp.Authorization = auth + gmpEp.Authorization = c.convertAuthorization(authorization) } // Handle deprecated BearerTokenSecret -> Authorization. @@ -715,14 +774,9 @@ func (c *conversionContext) applyAuthAndTLS( 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("bearerTokenSecret: %w", err) - } - gmpEp.Authorization = auth + gmpEp.Authorization = c.convertAuthorization(&pomonitoringv1.SafeAuthorization{Credentials: &tokenSecret}) } } - return nil } func convertMetricRelabelings( @@ -737,7 +791,7 @@ func convertMetricRelabelings( action = relabel.Replace } - if shouldSkipRelabelConfig(logger, config, action) { + if skip, _, _ := shouldSkipRelabelConfig(logger, config, action); skip { continue } @@ -831,34 +885,41 @@ func convertTargetLabels(logger *slog.Logger, sourceLabels []string, jobLabel st } // shouldSkipRelabelConfig checks if the relabel config uses unsupported actions or references annotations. -func shouldSkipRelabelConfig(logger *slog.Logger, config pomonitoringv1.RelabelConfig, action relabel.Action) bool { +func shouldSkipRelabelConfig(logger *slog.Logger, config pomonitoringv1.RelabelConfig, action relabel.Action) (skip bool, scopeExpanded bool, dropDetails string) { switch action { case relabel.Replace, relabel.HashMod, "": if config.TargetLabel == "" { logger.Warn(fmt.Sprintf("Relabeling rule uses 'action: %s' with an empty 'targetLabel', which is invalid in Prometheus and has been dropped.", action)) - return true + return true, false, "" } case relabel.Keep, relabel.Drop, relabel.LabelDrop, relabel.LabelKeep: // Supported actions that do not require targetLabel. case relabel.LabelMap, relabel.Lowercase, relabel.Uppercase, relabel.KeepEqual, relabel.DropEqual: logger.Warn(fmt.Sprintf("Relabeling rule uses 'action: %s' which is not supported by GMP and has been dropped.", action)) - return true + return true, false, "" default: logger.Warn(fmt.Sprintf("Relabeling rule uses unknown 'action: %s' which is not supported by GMP and has been dropped.", action)) - return true + return true, false, "" } for _, sl := range config.SourceLabels { - if strings.HasPrefix(string(sl), "__meta_kubernetes_pod_annotation_") { - logger.Warn(fmt.Sprintf("Relabeling rule referencing pod annotation %q is unsupported in GMP. The rule has been dropped.", string(sl))) - return true + s := string(sl) + if strings.HasPrefix(s, "__meta_kubernetes_pod_annotation_") { + logger.Warn(fmt.Sprintf("Relabeling rule referencing pod annotation %q is unsupported in GMP. The rule has been dropped.", s)) + if action == relabel.Keep || action == relabel.Drop { + return true, true, fmt.Sprintf("'%s' on '%s'", action, s) + } + return true, false, "" } - if strings.HasPrefix(string(sl), "__meta_kubernetes_node_") && string(sl) != "__meta_kubernetes_node_name" { - logger.Warn(fmt.Sprintf("Relabeling rule referencing node metadata %q is unsupported in GMP (only node name is supported). The rule has been dropped.", string(sl))) - return true + if strings.HasPrefix(s, "__meta_kubernetes_node_") && s != "__meta_kubernetes_node_name" { + logger.Warn(fmt.Sprintf("Relabeling rule referencing node metadata %q is unsupported in GMP (only node name is supported). The rule has been dropped.", s)) + if action == relabel.Keep || action == relabel.Drop { + return true, true, fmt.Sprintf("'%s' on '%s'", action, s) + } + return true, false, "" } } - return false + return false, false, "" } // resolveSourceLabels resolves source labels to pod labels, metadata labels, and rewritten labels. @@ -914,7 +975,12 @@ func convertRelabelingToSelector(logger *slog.Logger, data *relabelingData, isSi res.MatchLabels = make(map[string]string) } if existing, exists := res.MatchLabels[labelName]; exists && existing != parts[0] { - return false, fmt.Errorf("conflicting keep rules for label %q: cannot require both %q and %q simultaneously", labelName, existing, parts[0]) + res.Todos = append(res.Todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Conflicting relabeling keep rules for label %q: cannot require both %q and %q simultaneously.", labelName, existing, parts[0]), + action: fmt.Sprintf("Define the intended label value for %q in 'spec.selector.matchLabels'.", labelName), + }) + return true, nil } res.MatchLabels[labelName] = parts[0] logger.Info(fmt.Sprintf("Converted target filtering relabeling rule (%q -> %q) to Pod Selector (matchLabels).", source, parts[0])) @@ -1118,6 +1184,13 @@ func buildPodMonitoring( u.SetAPIVersion(GMPAPIVersion) u.SetKind(KindPodMonitoring) + for _, td := range spec.todos { + AddMigrationTodo(u, td.category, td.reason, td.action) + if logger != nil { + logger.Warn(td.reason, slog.String("action", td.action), slog.String("migration_status", "action_items")) + } + } + return u, nil } @@ -1155,6 +1228,13 @@ func buildClusterPodMonitoring( u.SetAPIVersion(GMPAPIVersion) u.SetKind(KindClusterPodMonitoring) + for _, td := range spec.todos { + AddMigrationTodo(u, td.category, td.reason, td.action) + if logger != nil { + logger.Warn(td.reason, slog.String("action", td.action), slog.String("migration_status", "action_items")) + } + } + return u, nil } @@ -1267,43 +1347,68 @@ 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) (resolvedInterval, resolvedTimeout string, err error) { +func (c *conversionContext) resolveScrapeIntervalAndTimeout(interval, timeout string) (resolvedInterval, resolvedTimeout string) { // 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.") + c.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) + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Invalid scrape interval %q.", interval), + action: "Specify a valid Prometheus duration string (e.g. '30s', '1m') in 'spec.endpoints[].interval'.", + }) + interval = "30s" + intDur, _ = prommodel.ParseDuration("30s") } // 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.", + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Invalid scrape timeout %q.", timeout), + action: "Specify a valid Prometheus duration string (e.g. '10s') in 'spec.endpoints[].timeout'.", + }) + timeout = "" + } else if toDur > intDur { + c.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 + return interval, timeout } -// convertProxyURL verifies proxy URL credentials. -func convertProxyURL(proxyURL *string) (string, error) { +// convertProxyURL verifies proxy URL credentials and attaches a TODO if credentials are present or malformed. +func (c *conversionContext) convertProxyURL(proxyURL *string) string { if proxyURL == nil { - return "", nil - } - if strings.Contains(*proxyURL, "@") { - return "", errors.New("proxyUrl contains credentials (matches '@'), which is blocked by GMP API validation") + return "" } - return *proxyURL, nil + parsed, err := url.Parse(*proxyURL) + if err != nil { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Proxy URL %q is invalid or malformed.", *proxyURL), + action: "Specify a valid proxy URL (e.g. 'http://proxy.example.com:8080').", + }) + return "TODO_SET_VALID_PROXY_URL" + } + if parsed.User != nil { + c.todos = append(c.todos, todoItem{ + category: "ERROR", + reason: "Proxy URL contains embedded plaintext credentials. Credentials were removed.", + action: "Configure proxy authentication via Kubernetes Secret or proxy server configuration.", + }) + parsed.User = nil + return parsed.String() + } + return *proxyURL } // warnUnsupportedEndpointFields logs warnings for fields that GMP does not support. @@ -1360,15 +1465,19 @@ func endpointPortKey(ep pomonitoringv1.Endpoint) string { } // resolveServicePort resolves a Service port to the backing Pod's target port. -func resolveServicePort(logger *slog.Logger, svc *corev1.Service, portStr string) (intstr.IntOrString, error) { +func resolveServicePort(logger *slog.Logger, svc *corev1.Service, portStr string) (intstr.IntOrString, *todoItem) { if portStr == "" { - return intstr.IntOrString{}, errors.New("port string cannot be empty") + return intstr.FromString("TODO_SET_PORT"), nil } if svc == nil { - return intstr.IntOrString{}, errors.New("cannot resolve port on nil or uninitialized Service") + return intstr.FromString("TODO_RESOLVE_PORT"), nil } if len(svc.Spec.Ports) == 0 { - return intstr.IntOrString{}, errors.New("service has no ports defined in spec") + return intstr.FromString(fmt.Sprintf("TODO_RESOLVE_PORT_%s", strings.ToUpper(portStr))), &todoItem{ + category: "WARNING", + reason: fmt.Sprintf("Port %q could not be resolved because Service %q defines no ports in its spec.", portStr, svc.Name), + action: fmt.Sprintf("Verify that target pods expose port %q.", portStr), + } } for _, p := range svc.Spec.Ports { @@ -1388,7 +1497,11 @@ func resolveServicePort(logger *slog.Logger, svc *corev1.Service, portStr string } } - return intstr.IntOrString{}, fmt.Errorf("port %q not found in Service spec", portStr) + return intstr.FromString(fmt.Sprintf("TODO_RESOLVE_PORT_%s", strings.ToUpper(portStr))), &todoItem{ + category: "WARNING", + reason: fmt.Sprintf("Port %q was not found in Service %q spec.", portStr, svc.Name), + action: fmt.Sprintf("Verify that target pods expose port %q.", portStr), + } } const ( diff --git a/pkg/migrate/helpers_test.go b/pkg/migrate/helpers_test.go index f91b6f9324..043ffee19f 100644 --- a/pkg/migrate/helpers_test.go +++ b/pkg/migrate/helpers_test.go @@ -85,14 +85,16 @@ func TestExtractSecretKey(t *testing.T) { setupCache func(cache *ResourceCache) error selector corev1.SecretKeySelector expectedVal string + expectTodos int wantErr bool }{ { name: "Missing secret", setupCache: func(_ *ResourceCache) error { return nil }, // Empty cache. selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "missing"}, Key: "user"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_USER_FROM_SECRET_MISSING", + expectTodos: 1, + wantErr: false, }, { name: "Secret with StringData", @@ -101,6 +103,7 @@ func TestExtractSecretKey(t *testing.T) { }, selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret"}, Key: "user"}, expectedVal: "admin", + expectTodos: 0, wantErr: false, }, { @@ -110,21 +113,24 @@ func TestExtractSecretKey(t *testing.T) { }, selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret-2"}, Key: "pass"}, expectedVal: "supersecret", + expectTodos: 0, wantErr: false, }, { name: "Secret reference with empty Name", setupCache: func(_ *ResourceCache) error { return nil }, selector: corev1.SecretKeySelector{Key: "user"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_USER_FROM_SECRET_EMPTY_NAME", + expectTodos: 1, + wantErr: false, }, { name: "Secret reference with empty Key", setupCache: func(_ *ResourceCache) error { return nil }, selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret"}}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_EMPTY_KEY_FROM_SECRET_MY-SECRET", + expectTodos: 1, + wantErr: false, }, { name: "Secret exists but key missing", @@ -132,8 +138,9 @@ func TestExtractSecretKey(t *testing.T) { return addSecretToCache(cache, "default", "my-secret", "user", "admin", true) }, selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret"}, Key: "password"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_MISSING_KEY_PASSWORD_IN_SECRET_MY-SECRET", + expectTodos: 1, + wantErr: false, }, { name: "Secret exists but data corrupted", @@ -154,8 +161,9 @@ func TestExtractSecretKey(t *testing.T) { return cache.Add(secret) }, selector: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "corrupted-secret"}, Key: "pass"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_CORRUPT_SECRET_DATA_PASS", + expectTodos: 1, + wantErr: false, }, } @@ -166,13 +174,13 @@ func TestExtractSecretKey(t *testing.T) { t.Fatalf("failed to setup cache: %v", err) } - val, err := ctx.extractSecretKey(tc.selector) - if (err != nil) != tc.wantErr { - t.Fatalf("extractSecretKey() error = %v, wantErr %v", err, tc.wantErr) - } - if !tc.wantErr && val != tc.expectedVal { + val := ctx.extractSecretKey(tc.selector) + if val != tc.expectedVal { t.Errorf("expected %s, got %s", tc.expectedVal, val) } + if len(ctx.todos) != tc.expectTodos { + t.Errorf("expected %d todos, got %d", tc.expectTodos, len(ctx.todos)) + } }) } } @@ -183,14 +191,16 @@ func TestExtractConfigMapKey(t *testing.T) { setupCache func(cache *ResourceCache) error selector corev1.ConfigMapKeySelector expectedVal string + expectTodos int wantErr bool }{ { name: "Missing configmap", setupCache: func(_ *ResourceCache) error { return nil }, // Empty cache. selector: corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "missing"}, Key: "user"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_USER_FROM_CONFIGMAP_MISSING", + expectTodos: 1, + wantErr: false, }, { name: "Found configmap", @@ -199,21 +209,24 @@ func TestExtractConfigMapKey(t *testing.T) { }, selector: corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-cm"}, Key: "id"}, expectedVal: "client-123", + expectTodos: 0, wantErr: false, }, { name: "Configmap reference with empty Name", setupCache: func(_ *ResourceCache) error { return nil }, selector: corev1.ConfigMapKeySelector{Key: "user"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_USER_FROM_CONFIGMAP_EMPTY_NAME", + expectTodos: 1, + wantErr: false, }, { name: "Configmap reference with empty Key", setupCache: func(_ *ResourceCache) error { return nil }, selector: corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-cm"}}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_SET_EMPTY_KEY_FROM_CONFIGMAP_MY-CM", + expectTodos: 1, + wantErr: false, }, { name: "Configmap exists but key missing", @@ -221,8 +234,9 @@ func TestExtractConfigMapKey(t *testing.T) { return addConfigMapToCache(cache, "default", "my-cm", "id", "client-123") }, selector: corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "my-cm"}, Key: "secret"}, - expectedVal: "", - wantErr: true, + expectedVal: "TODO_MISSING_KEY_SECRET_IN_CONFIGMAP_MY-CM", + expectTodos: 1, + wantErr: false, }, } @@ -233,13 +247,13 @@ func TestExtractConfigMapKey(t *testing.T) { t.Fatalf("failed to setup cache: %v", err) } - val, err := ctx.extractConfigMapKey(tc.selector) - if (err != nil) != tc.wantErr { - t.Fatalf("extractConfigMapKey() error = %v, wantErr %v", err, tc.wantErr) - } - if !tc.wantErr && val != tc.expectedVal { + val := ctx.extractConfigMapKey(tc.selector) + if val != tc.expectedVal { t.Errorf("expected %s, got %s", tc.expectedVal, val) } + if len(ctx.todos) != tc.expectTodos { + t.Errorf("expected %d todos, got %d", tc.expectTodos, len(ctx.todos)) + } }) } } @@ -281,19 +295,19 @@ func TestConvertConfigMapToSecretSelector(t *testing.T) { name: "Empty name reference", setupCache: func(_ *ResourceCache) error { return nil }, selector: &corev1.ConfigMapKeySelector{Key: "ca.crt"}, - expectedSecretName: "", - expectedSecretKey: "", + expectedSecretName: "secret-TODO_SET_CONFIGMAP_NAME", + expectedSecretKey: "ca.crt", expectGeneratedSecret: false, - wantErr: true, + wantErr: false, }, { name: "Empty key reference", setupCache: func(_ *ResourceCache) error { return nil }, selector: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "tls-cm"}}, - expectedSecretName: "", - expectedSecretKey: "", + expectedSecretName: "secret-tls-cm", + expectedSecretKey: "TODO_SET_CONFIGMAP_KEY", expectGeneratedSecret: false, - wantErr: true, + wantErr: false, }, } @@ -304,10 +318,7 @@ func TestConvertConfigMapToSecretSelector(t *testing.T) { t.Fatalf("failed to setup cache: %v", err) } - gmpSel, err := ctx.convertConfigMapToSecretSelector(tc.selector) - if (err != nil) != tc.wantErr { - t.Fatalf("convertConfigMapToSecretSelector() error = %v, wantErr %v", err, tc.wantErr) - } + gmpSel := ctx.convertConfigMapToSecretSelector(tc.selector) if tc.selector == nil { if gmpSel != nil { @@ -315,9 +326,6 @@ func TestConvertConfigMapToSecretSelector(t *testing.T) { } return } - if tc.wantErr { - return - } if gmpSel == nil || gmpSel.Secret == nil || gmpSel.Secret.Name != tc.expectedSecretName || gmpSel.Secret.Key != tc.expectedSecretKey { t.Errorf("unexpected secret selector: %+v", gmpSel) @@ -368,13 +376,18 @@ func TestConvertBasicAuth(t *testing.T) { wantErr: false, }, { - name: "Malformed Username reference", - setupCache: func(_ *ResourceCache) error { return nil }, + name: "Malformed Username reference", + setupCache: func(cache *ResourceCache) error { + return addSecretToCache(cache, "default", "auth-secret", "pass", "pass123", true) + }, basicAuth: &pomonitoringv1.BasicAuth{ Username: corev1.SecretKeySelector{Key: "user"}, Password: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "auth-secret"}, Key: "pass"}, }, - wantErr: true, + expectedUser: "TODO_SET_USER_FROM_SECRET_EMPTY_NAME", + expectedPassName: "auth-secret", + expectedPassKey: "pass", + wantErr: false, }, } @@ -385,10 +398,7 @@ func TestConvertBasicAuth(t *testing.T) { t.Fatalf("failed to setup cache: %v", err) } - gmpBA, err := ctx.convertBasicAuth(tc.basicAuth) - if (err != nil) != tc.wantErr { - t.Fatalf("convertBasicAuth() error = %v, wantErr %v", err, tc.wantErr) - } + gmpBA := ctx.convertBasicAuth(tc.basicAuth) if tc.basicAuth == nil { if gmpBA != nil { @@ -396,9 +406,6 @@ func TestConvertBasicAuth(t *testing.T) { } return } - if tc.wantErr { - return - } if gmpBA.Username != tc.expectedUser { t.Errorf("expected username %s, got %s", tc.expectedUser, gmpBA.Username) @@ -461,7 +468,9 @@ func TestConvertSafeTLSConfig(t *testing.T) { ConfigMap: &corev1.ConfigMapKeySelector{Key: "ca.crt"}, }, }, - wantErr: true, + expectedCAName: "secret-TODO_SET_CONFIGMAP_NAME", + expectedCAKey: "ca.crt", + wantErr: false, }, } @@ -472,10 +481,7 @@ func TestConvertSafeTLSConfig(t *testing.T) { t.Fatalf("failed to setup cache: %v", err) } - gmpTLS, err := ctx.convertSafeTLSConfig(tc.tlsConfig) - if (err != nil) != tc.wantErr { - t.Fatalf("convertSafeTLSConfig() error = %v, wantErr %v", err, tc.wantErr) - } + gmpTLS := ctx.convertSafeTLSConfig(tc.tlsConfig) if tc.tlsConfig == nil { if gmpTLS != nil { @@ -483,15 +489,16 @@ func TestConvertSafeTLSConfig(t *testing.T) { } return } - if tc.wantErr { - return - } - if gmpTLS.CA.Secret.Name != tc.expectedCAName || gmpTLS.CA.Secret.Key != tc.expectedCAKey { - t.Errorf("unexpected CA selector: %+v", gmpTLS.CA) + if tc.expectedCAName != "" { + if gmpTLS.CA == nil || gmpTLS.CA.Secret == nil || gmpTLS.CA.Secret.Name != tc.expectedCAName || gmpTLS.CA.Secret.Key != tc.expectedCAKey { + t.Errorf("unexpected CA selector: %+v", gmpTLS.CA) + } } - if gmpTLS.Cert.Secret.Name != tc.expectedCertName || gmpTLS.Cert.Secret.Key != tc.expectedCertKey { - t.Errorf("unexpected Cert selector: %+v", gmpTLS.Cert) + if tc.expectedCertName != "" { + if gmpTLS.Cert == nil || gmpTLS.Cert.Secret == nil || gmpTLS.Cert.Secret.Name != tc.expectedCertName || gmpTLS.Cert.Secret.Key != tc.expectedCertKey { + t.Errorf("unexpected Cert selector: %+v", gmpTLS.Cert) + } } if gmpTLS.InsecureSkipVerify != tc.expectedSkipVerify { t.Errorf("expected InsecureSkipVerify %v, got %v", tc.expectedSkipVerify, gmpTLS.InsecureSkipVerify) @@ -503,6 +510,118 @@ func TestConvertSafeTLSConfig(t *testing.T) { } } +func TestConvertOAuth2(t *testing.T) { + tests := []struct { + name string + setupCache func(cache *ResourceCache) error + oauth2 *pomonitoringv1.OAuth2 + expectedClientID string + expectedSecName string + expectedSecKey string + expectedTokenURL string + expectTodos int + }{ + { + name: "Nil OAuth2 returns nil", + setupCache: func(_ *ResourceCache) error { return nil }, + oauth2: nil, + }, + { + name: "Valid OAuth2 from Secret", + setupCache: func(cache *ResourceCache) error { + return addSecretToCache(cache, "default", "oauth-sec", "client_id", "my-client", true) + }, + oauth2: &pomonitoringv1.OAuth2{ + ClientID: pomonitoringv1.SecretOrConfigMap{ + Secret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oauth-sec"}, + Key: "client_id", + }, + }, + ClientSecret: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oauth-sec"}, + Key: "client_secret", + }, + TokenURL: "https://auth.example.com/token", + }, + expectedClientID: "my-client", + expectedSecName: "oauth-sec", + expectedSecKey: "client_secret", + expectedTokenURL: "https://auth.example.com/token", + expectTodos: 0, + }, + { + name: "Empty ClientID generates placeholder and TODO", + setupCache: func(_ *ResourceCache) error { return nil }, + oauth2: &pomonitoringv1.OAuth2{ + ClientSecret: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oauth-sec"}, + Key: "client_secret", + }, + TokenURL: "https://auth.example.com/token", + }, + expectedClientID: "TODO_SET_OAUTH2_CLIENT_ID", + expectedSecName: "oauth-sec", + expectedSecKey: "client_secret", + expectedTokenURL: "https://auth.example.com/token", + expectTodos: 1, + }, + { + name: "Empty TokenURL generates placeholder and TODO", + setupCache: func(cache *ResourceCache) error { + return addSecretToCache(cache, "default", "oauth-sec", "client_id", "my-client", true) + }, + oauth2: &pomonitoringv1.OAuth2{ + ClientID: pomonitoringv1.SecretOrConfigMap{ + Secret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oauth-sec"}, + Key: "client_id", + }, + }, + ClientSecret: corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oauth-sec"}, + Key: "client_secret", + }, + TokenURL: "", + }, + expectedClientID: "my-client", + expectedSecName: "oauth-sec", + expectedSecKey: "client_secret", + expectedTokenURL: "TODO_SET_OAUTH2_TOKEN_URL", + expectTodos: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := newTestConversionContext() + if err := tc.setupCache(ctx.cache); err != nil { + t.Fatalf("failed to setup cache: %v", err) + } + + res := ctx.convertOAuth2(tc.oauth2) + if tc.oauth2 == nil { + if res != nil { + t.Errorf("expected nil result for nil OAuth2, got %+v", res) + } + return + } + if res.ClientID != tc.expectedClientID { + t.Errorf("expected ClientID %q, got %q", tc.expectedClientID, res.ClientID) + } + if res.TokenURL != tc.expectedTokenURL { + t.Errorf("expected TokenURL %q, got %q", tc.expectedTokenURL, res.TokenURL) + } + if res.ClientSecret.Secret == nil || res.ClientSecret.Secret.Name != tc.expectedSecName || res.ClientSecret.Secret.Key != tc.expectedSecKey { + t.Errorf("unexpected ClientSecret selector: %+v", res.ClientSecret) + } + if len(ctx.todos) != tc.expectTodos { + t.Errorf("expected %d todos, got %d", tc.expectTodos, len(ctx.todos)) + } + }) + } +} + func TestConvertConfigMapToSecretSelectorDeduplication(t *testing.T) { ctx := newTestConversionContext() err := addConfigMapToCache(ctx.cache, "default", "tls-cm", "ca.crt", "cert-data") @@ -516,19 +635,13 @@ func TestConvertConfigMapToSecretSelectorDeduplication(t *testing.T) { } // Call first time. - gmpSel1, err := ctx.convertConfigMapToSecretSelector(selector) - if err != nil { - t.Fatalf("first call failed with error: %v", err) - } + gmpSel1 := ctx.convertConfigMapToSecretSelector(selector) if gmpSel1 == nil || gmpSel1.Secret.Name != "secret-tls-cm" { t.Fatal("first call failed to translate selector") } // Call second time. - gmpSel2, err := ctx.convertConfigMapToSecretSelector(selector) - if err != nil { - t.Fatalf("second call failed with error: %v", err) - } + gmpSel2 := ctx.convertConfigMapToSecretSelector(selector) if gmpSel2 == nil || gmpSel2.Secret.Name != "secret-tls-cm" { t.Fatal("second call failed to translate selector") } @@ -670,14 +783,13 @@ func TestDetermineNamespaceScoping(t *testing.T) { } 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 + expectTodos int }{ { name: "empty defaults to 30s", @@ -685,7 +797,7 @@ func TestResolveScrapeIntervalAndTimeout(t *testing.T) { timeout: "", expectedInt: "30s", expectedTimeout: "", - expectErr: false, + expectTodos: 0, }, { name: "valid interval and timeout", @@ -693,7 +805,7 @@ func TestResolveScrapeIntervalAndTimeout(t *testing.T) { timeout: "10s", expectedInt: "15s", expectedTimeout: "10s", - expectErr: false, + expectTodos: 0, }, { name: "timeout larger than interval is capped", @@ -701,36 +813,38 @@ func TestResolveScrapeIntervalAndTimeout(t *testing.T) { timeout: "20s", expectedInt: "10s", expectedTimeout: "10s", - expectErr: false, + expectTodos: 0, }, { - name: "invalid interval duration", - interval: "invalid", - expectErr: true, + name: "invalid interval duration defaults to 30s with todo", + interval: "invalid", + expectedInt: "30s", + expectedTimeout: "", + expectTodos: 1, }, { - name: "invalid timeout duration", - interval: "15s", - timeout: "invalid", - expectErr: true, + name: "invalid timeout duration is dropped with todo", + interval: "15s", + timeout: "invalid", + expectedInt: "15s", + expectedTimeout: "", + expectTodos: 1, }, } 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 - } + ctx := newTestConversionContext() + intVal, toVal := ctx.resolveScrapeIntervalAndTimeout(tc.interval, tc.timeout) 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) } + if len(ctx.todos) != tc.expectTodos { + t.Errorf("expected %d todos, got %d", tc.expectTodos, len(ctx.todos)) + } }) } } @@ -740,39 +854,53 @@ func TestConvertProxyURL(t *testing.T) { name string proxyURL *string expectedURL string + expectTodos int expectErr bool }{ { name: "nil proxyURL", proxyURL: nil, expectedURL: "", + expectTodos: 0, expectErr: false, }, { name: "valid proxyURL without credentials", proxyURL: ptrTo("http://proxy.example.com"), expectedURL: "http://proxy.example.com", + expectTodos: 0, expectErr: false, }, { - name: "proxyURL with credentials returns error", - proxyURL: ptrTo("http://user:pass@proxy.example.com"), - expectErr: true, + name: "proxyURL with credentials sanitizes password and adds todo", + proxyURL: ptrTo("http://user:pass@proxy.example.com:8080"), + expectedURL: "http://proxy.example.com:8080", + expectTodos: 1, + }, + { + name: "proxyURL with username only sanitizes credentials and adds todo", + proxyURL: ptrTo("http://user@proxy.example.com:8080"), + expectedURL: "http://proxy.example.com:8080", + expectTodos: 1, + }, + { + name: "malformed proxyURL returns placeholder and adds todo", + proxyURL: ptrTo("://invalid-url"), + expectedURL: "TODO_SET_VALID_PROXY_URL", + expectTodos: 1, }, } 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 - } + convCtx := &conversionContext{} + url := convCtx.convertProxyURL(tc.proxyURL) if url != tc.expectedURL { t.Errorf("convertProxyURL() = %v, want %v", url, tc.expectedURL) } + if len(convCtx.todos) != tc.expectTodos { + t.Errorf("expected %d todos, got %d", tc.expectTodos, len(convCtx.todos)) + } }) } } @@ -936,10 +1064,7 @@ func TestDecoupledNamespaces(t *testing.T) { LocalObjectReference: corev1.LocalObjectReference{Name: "my-secret"}, Key: "user", } - val, err := ctx.extractSecretKey(sel) - if err != nil { - t.Fatalf("extractSecretKey() unexpected error: %v", err) - } + val := ctx.extractSecretKey(sel) if val != "admin" { t.Errorf("extractSecretKey() = %q, want %q", val, "admin") } @@ -952,10 +1077,7 @@ func TestDecoupledNamespaces(t *testing.T) { LocalObjectReference: corev1.LocalObjectReference{Name: "tls-cm"}, Key: "ca.crt", } - secretSel, err := ctx.convertConfigMapToSecretSelector(cmSel) - if err != nil { - t.Fatalf("convertConfigMapToSecretSelector() unexpected error: %v", err) - } + secretSel := ctx.convertConfigMapToSecretSelector(cmSel) if secretSel.Secret.Namespace != "target-ns" { t.Errorf("expected selector namespace %q, got %q", "target-ns", secretSel.Secret.Namespace) } @@ -1082,81 +1204,81 @@ func TestFindServicesBySelector(t *testing.T) { func TestResolveServicePort(t *testing.T) { tests := []struct { - name string - service *corev1.Service - portStr string - expected intstr.IntOrString - wantErr bool + name string + service *corev1.Service + portStr string + expected intstr.IntOrString + expectTodo bool }{ { name: "Resolve by name to int", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromInt32(8080)}, }), - portStr: "web", - expected: intstr.FromInt32(8080), - wantErr: false, + portStr: "web", + expected: intstr.FromInt32(8080), + expectTodo: false, }, { name: "Resolve by name to string", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromString("http-web")}, }), - portStr: "web", - expected: intstr.FromString("http-web"), - wantErr: false, + portStr: "web", + expected: intstr.FromString("http-web"), + expectTodo: false, }, { name: "Resolve by port number to targetPort int", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromInt32(8080)}, }), - portStr: "80", - expected: intstr.FromInt32(8080), - wantErr: false, + portStr: "80", + expected: intstr.FromInt32(8080), + expectTodo: false, }, { name: "Resolve by targetPort string when port name omitted", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromString("http-metrics")}, }), - portStr: "http-metrics", - expected: intstr.FromString("http-metrics"), - wantErr: false, + portStr: "http-metrics", + expected: intstr.FromString("http-metrics"), + expectTodo: false, }, { name: "Resolve by port number", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromString("http-web")}, }), - portStr: "80", - expected: intstr.FromString("http-web"), - wantErr: false, + portStr: "80", + expected: intstr.FromString("http-web"), + expectTodo: false, }, { name: "Resolve with omitted targetPort", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80}, }), - portStr: "web", - expected: intstr.FromInt32(80), - wantErr: false, + portStr: "web", + expected: intstr.FromInt32(80), + expectTodo: false, }, { name: "Port not found", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80}, }), - portStr: "admin", - expected: intstr.IntOrString{}, - wantErr: true, + portStr: "admin", + expected: intstr.FromString("TODO_RESOLVE_PORT_ADMIN"), + expectTodo: true, }, { - name: "Nil service", - service: nil, - portStr: "web", - expected: intstr.IntOrString{}, - wantErr: true, + name: "Nil service", + service: nil, + portStr: "web", + expected: intstr.FromString("TODO_RESOLVE_PORT"), + expectTodo: false, }, { name: "Skip malformed port entry and resolve valid later entry", @@ -1169,12 +1291,12 @@ func TestResolveServicePort(t *testing.T) { }, }, }, - portStr: "web", - expected: intstr.FromInt32(8080), - wantErr: false, + portStr: "web", + expected: intstr.FromInt32(8080), + expectTodo: false, }, { - name: "All ports malformed returns error", + name: "All ports malformed returns todo placeholder", service: &corev1.Service{ ObjectMeta: metav1.ObjectMeta{Name: "my-svc", Namespace: "default"}, Spec: corev1.ServiceSpec{ @@ -1184,9 +1306,9 @@ func TestResolveServicePort(t *testing.T) { }, }, }, - portStr: "web", - expected: intstr.IntOrString{}, - wantErr: true, + portStr: "web", + expected: intstr.FromString("TODO_RESOLVE_PORT_WEB"), + expectTodo: true, }, { name: "Out of range port number is rejected", @@ -1200,18 +1322,18 @@ func TestResolveServicePort(t *testing.T) { }, }, }, - portStr: "web", - expected: intstr.FromInt32(8080), - wantErr: false, + portStr: "web", + expected: intstr.FromInt32(8080), + expectTodo: false, }, { name: "Resolve with empty string targetPort defaults to port number", service: makeTestTypedService("default", "my-svc", nil, []corev1.ServicePort{ {Name: "web", Port: 80, TargetPort: intstr.FromString("")}, }), - portStr: "web", - expected: intstr.FromInt32(80), - wantErr: false, + portStr: "web", + expected: intstr.FromInt32(80), + expectTodo: false, }, } @@ -1219,12 +1341,9 @@ func TestResolveServicePort(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := resolveServicePort(logger, tc.service, tc.portStr) - if (err != nil) != tc.wantErr { - t.Fatalf("resolveServicePort() error = %v, wantErr %v", err, tc.wantErr) - } - if tc.wantErr { - return + got, todo := resolveServicePort(logger, tc.service, tc.portStr) + if (todo != nil) != tc.expectTodo { + t.Fatalf("resolveServicePort() todo = %v, expectTodo %v", todo, tc.expectTodo) } if got != tc.expected { t.Errorf("expected %+v, got %+v", tc.expected, got) @@ -1314,3 +1433,56 @@ func TestMakeUniqueResourceName(t *testing.T) { }) } } + +func TestAddMigrationTodo(t *testing.T) { + tests := []struct { + name string + initialObj map[string]any + category string + reason string + action string + expectedMap map[string]string + }{ + { + name: "single todo with action", + initialObj: map[string]any{ + "metadata": map[string]any{}, + }, + category: "WARNING", + reason: "Dropped unsupported 'annotationMatches' selector.", + action: "Verify 'spec.selector.matchLabels' on target pods and remove guardrail label.", + expectedMap: map[string]string{ + "gmp.googleapis.com/todo-1": "[WARNING] Dropped unsupported 'annotationMatches' selector. ACTION: Verify 'spec.selector.matchLabels' on target pods and remove guardrail label.", + }, + }, + { + name: "sequential todos preserving existing annotations", + initialObj: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{ + "existing.io/key": "existing-val", + "gmp.googleapis.com/todo-1": "[WARNING] First todo. ACTION: Fix first item.", + }, + }, + }, + category: "ERROR", + reason: "Invalid proxy URL.", + action: "Move credentials to Secret.", + expectedMap: map[string]string{ + "existing.io/key": "existing-val", + "gmp.googleapis.com/todo-1": "[WARNING] First todo. ACTION: Fix first item.", + "gmp.googleapis.com/todo-2": "[ERROR] Invalid proxy URL. ACTION: Move credentials to Secret.", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + u := &unstructured.Unstructured{Object: tc.initialObj} + AddMigrationTodo(u, tc.category, tc.reason, tc.action) + if diff := cmp.Diff(tc.expectedMap, u.GetAnnotations()); diff != "" { + t.Errorf("AddMigrationTodo() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/migrate/logger.go b/pkg/migrate/logger.go index bcd2b1527b..008752c719 100644 --- a/pkg/migrate/logger.go +++ b/pkg/migrate/logger.go @@ -29,18 +29,13 @@ import ( type ResourceStatus int const ( - StatusSuccess ResourceStatus = iota // 0 (Migrated Successfully). - StatusSkipped // 1 (Skipped / Unsupported). - StatusWarning // 2 (Migrated with Warnings). - StatusFailed // 3 (Failed). + StatusSuccess ResourceStatus = iota // 0 (Migrated Successfully). + StatusSkipped // 1 (Skipped / Unsupported). + StatusWarnings // 2 (Migrated with Warnings). + StatusActionItems // 3 (Migrated with Action Items). + StatusFailed // 4 (Failed). ) -// statusLevels maps slog.Levels to their corresponding ResourceStatus. -var statusLevels = map[slog.Level]ResourceStatus{ - slog.LevelWarn: StatusWarning, - slog.LevelError: StatusFailed, -} - // loggerState encapsulates the shared, thread-safe state across all handler clones. type loggerState struct { mu sync.Mutex @@ -164,15 +159,23 @@ func (h *ConsoleHandler) Handle(_ context.Context, r slog.Record) error { } if key != "" { - if r.Level == slog.LevelInfo { + switch r.Level { + case slog.LevelInfo: switch migrationStatus { case "skipped": h.trackStatus(key, StatusSkipped) case "success": h.trackStatus(key, StatusSuccess) } - } else if status, ok := statusLevels[r.Level]; ok { - h.trackStatus(key, status) + case slog.LevelWarn: + switch migrationStatus { + case "action_items": + h.trackStatus(key, StatusActionItems) + default: + h.trackStatus(key, StatusWarnings) + } + case slog.LevelError: + h.trackStatus(key, StatusFailed) } } diff --git a/pkg/migrate/migrate.go b/pkg/migrate/migrate.go index 4d19f340a8..4d329e1eb4 100644 --- a/pkg/migrate/migrate.go +++ b/pkg/migrate/migrate.go @@ -37,11 +37,13 @@ import ( // MigrationReport accumulates the statistics and payloads of the migration run. type MigrationReport struct { - SuccessCount int // Successfully migrated with no warnings. - WarningCount int // Successfully migrated but had warnings. - SkippedCount int // Bypassed because resource is unsupported/out-of-scope. - FailedCount int // Fatal failure, resource skipped. - Outputs []*unstructured.Unstructured // Converted GMP manifests in-memory. + SuccessCount int // Successfully migrated with no warnings or action items. + WarningsCount int // Migrated with non-blocking warnings (e.g. dropped unsupported fields). + ActionItemsCount int // Successfully migrated but had TODO annotations. + SkippedCount int // Bypassed because resource is unsupported/out-of-scope. + FailedCount int // Fatal failure, resource skipped. + Outputs []*unstructured.Unstructured // All converted GMP manifests in-memory. + ReadyOutputs []*unstructured.Unstructured // Only 100% ready manifests (0 TODO annotations). } // Migrator orchestrates the migration process. @@ -120,6 +122,14 @@ func (m *Migrator) Run(inputPaths ...string) (*MigrationReport, error) { outputs := m.convertResources() report.Outputs = outputs + var readyOutputs []*unstructured.Unstructured + for _, out := range outputs { + if !hasTodoAnnotations(out) { + readyOutputs = append(readyOutputs, out) + } + } + report.ReadyOutputs = readyOutputs + // 4. Calculate final statistics from the handler's tracked statuses. for _, status := range handler.ResourceStatuses() { switch status { @@ -127,8 +137,10 @@ func (m *Migrator) Run(inputPaths ...string) (*MigrationReport, error) { report.SuccessCount++ case StatusSkipped: report.SkippedCount++ - case StatusWarning: - report.WarningCount++ + case StatusWarnings: + report.WarningsCount++ + case StatusActionItems: + report.ActionItemsCount++ case StatusFailed: report.FailedCount++ } @@ -137,15 +149,39 @@ func (m *Migrator) Run(inputPaths ...string) (*MigrationReport, error) { return report, nil } +// hasTodoAnnotations returns true if the resource contains any GMP migration TODO annotations. +func hasTodoAnnotations(u *unstructured.Unstructured) bool { + if u == nil { + return false + } + for k := range u.GetAnnotations() { + if strings.HasPrefix(k, AnnotationTodoPrefix) { + return true + } + } + return false +} + // PrintSummary formats and writes the standardized migration report summary. -func (m *Migrator) PrintSummary(r *MigrationReport) { +func (m *Migrator) PrintSummary(r *MigrationReport, emitAll bool) { fmt.Fprintln(m.Stderr, "\n=========================================") fmt.Fprintln(m.Stderr, "Migration Complete Summary:") - fmt.Fprintf(m.Stderr, " Successfully Migrated: %d\n", r.SuccessCount) - fmt.Fprintf(m.Stderr, " Migrated with Warnings: %d\n", r.WarningCount) - fmt.Fprintf(m.Stderr, " Skipped (Unsupported): %d\n", r.SkippedCount) - fmt.Fprintf(m.Stderr, " Failed: %d\n", r.FailedCount) + fmt.Fprintf(m.Stderr, " Successfully Migrated: %d\n", r.SuccessCount) + fmt.Fprintf(m.Stderr, " Migrated with Warnings: %d\n", r.WarningsCount) + fmt.Fprintf(m.Stderr, " Migrated with Action Items: %d\n", r.ActionItemsCount) + fmt.Fprintf(m.Stderr, " Skipped (Unsupported): %d\n", r.SkippedCount) + fmt.Fprintf(m.Stderr, " Failed: %d\n", r.FailedCount) fmt.Fprintln(m.Stderr, "=========================================") + if r.ActionItemsCount > 0 { + if !emitAll { + fmt.Fprintf(m.Stderr, "\nNOTE: Emitted %d ready manifests to Stdout.\n", len(r.ReadyOutputs)) + fmt.Fprintf(m.Stderr, "%d manifests with action items were omitted from Stdout as they contain best-effort draft configurations with TODO annotations and placeholders.\n", r.ActionItemsCount) + fmt.Fprintln(m.Stderr, "Run with '--all' to output all manifests for review.") + } else { + fmt.Fprintf(m.Stderr, "\nNOTE: %d manifests contain best-effort draft configurations with TODO annotations and placeholders.\n", r.ActionItemsCount) + fmt.Fprintln(m.Stderr, "Review the inline 'gmp.googleapis.com/todo-*' annotations in the generated manifests before applying to a cluster.") + } + } } // WriteOutputs serializes and writes the converted manifests to the migrator's Stdout stream. @@ -359,6 +395,10 @@ func (m *Migrator) convertResources() []*unstructured.Unstructured { continue } + if len(outputs) == 0 { + continue + } + resourceLogger.Info("Converted successfully", slog.String("migration_status", "success")) } } diff --git a/pkg/migrate/migrate_test.go b/pkg/migrate/migrate_test.go index 78db6829eb..f4f39c382a 100644 --- a/pkg/migrate/migrate_test.go +++ b/pkg/migrate/migrate_test.go @@ -103,8 +103,8 @@ spec: if report.SuccessCount != 1 { t.Errorf("expected SuccessCount to be 1, got %d", report.SuccessCount) } - if report.WarningCount != 0 { - t.Errorf("expected WarningCount to be 0, got %d", report.WarningCount) + if report.ActionItemsCount != 0 { + t.Errorf("expected ActionItemsCount to be 0, got %d", report.ActionItemsCount) } if report.SkippedCount != 0 { t.Errorf("expected SkippedCount to be 0, got %d", report.SkippedCount) @@ -210,8 +210,8 @@ spec: if report.SuccessCount != 0 { t.Errorf("expected SuccessCount to be 0, got %d", report.SuccessCount) } - if report.WarningCount != 0 { - t.Errorf("expected WarningCount to be 0, got %d", report.WarningCount) + if report.ActionItemsCount != 0 { + t.Errorf("expected ActionItemsCount to be 0, got %d", report.ActionItemsCount) } if report.SkippedCount != 0 { t.Errorf("expected SkippedCount to be 0, got %d", report.SkippedCount) @@ -263,8 +263,8 @@ spec: if report.SuccessCount != 0 { t.Errorf("expected SuccessCount to be 0, got %d", report.SuccessCount) } - if report.WarningCount != 0 { - t.Errorf("expected WarningCount to be 0, got %d", report.WarningCount) + if report.ActionItemsCount != 0 { + t.Errorf("expected ActionItemsCount to be 0, got %d", report.ActionItemsCount) } if report.FailedCount != 0 { t.Errorf("expected FailedCount to be 0, got %d", report.FailedCount) @@ -333,8 +333,8 @@ spec: if report.SuccessCount != 1 { t.Errorf("expected SuccessCount to be 1, got %d", report.SuccessCount) } - if report.WarningCount != 0 { - t.Errorf("expected WarningCount to be 0, got %d", report.WarningCount) + if report.ActionItemsCount != 0 { + t.Errorf("expected ActionItemsCount to be 0, got %d", report.ActionItemsCount) } if report.SkippedCount != 0 { t.Errorf("expected SkippedCount to be 0, got %d", report.SkippedCount) @@ -399,8 +399,8 @@ items: if report.SuccessCount != 1 { t.Errorf("expected SuccessCount to be 1, got %d", report.SuccessCount) } - if report.WarningCount != 0 { - t.Errorf("expected WarningCount to be 0, got %d", report.WarningCount) + if report.ActionItemsCount != 0 { + t.Errorf("expected ActionItemsCount to be 0, got %d", report.ActionItemsCount) } if report.SkippedCount != 0 { t.Errorf("expected SkippedCount to be 0, got %d", report.SkippedCount) @@ -415,3 +415,211 @@ items: t.Errorf("expected reference to be successfully resolved inside list, got logs: %q", stderrLogs) } } + +func TestMigratorPrintSummary(t *testing.T) { + tests := []struct { + name string + report *MigrationReport + emitAll bool + wantContains []string + }{ + { + name: "Clean report without action items", + report: &MigrationReport{ + SuccessCount: 2, + }, + emitAll: false, + wantContains: []string{ + "Successfully Migrated: 2", + "Migrated with Warnings: 0", + "Migrated with Action Items: 0", + }, + }, + { + name: "Default mode with action items notes omitted drafts", + report: &MigrationReport{ + SuccessCount: 1, + WarningsCount: 1, + ActionItemsCount: 1, + ReadyOutputs: make([]*unstructured.Unstructured, 2), + }, + emitAll: false, + wantContains: []string{ + "Successfully Migrated: 1", + "Migrated with Warnings: 1", + "Migrated with Action Items: 1", + "NOTE: Emitted 2 ready manifests to Stdout.", + "1 manifests with action items were omitted from Stdout as they contain best-effort draft configurations with TODO annotations and placeholders.", + "Run with '--all' to output all manifests for review.", + }, + }, + { + name: "All mode with action items notes draft review", + report: &MigrationReport{ + SuccessCount: 1, + WarningsCount: 1, + ActionItemsCount: 1, + Outputs: make([]*unstructured.Unstructured, 3), + }, + emitAll: true, + wantContains: []string{ + "Successfully Migrated: 1", + "Migrated with Warnings: 1", + "Migrated with Action Items: 1", + "NOTE: 1 manifests contain best-effort draft configurations with TODO annotations and placeholders.", + "Review the inline 'gmp.googleapis.com/todo-*' annotations in the generated manifests before applying to a cluster.", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + m := NewMigrator() + m.Stderr = &buf + m.PrintSummary(tc.report, tc.emitAll) + output := buf.String() + for _, s := range tc.wantContains { + if !strings.Contains(output, s) { + t.Errorf("PrintSummary output missing %q, got:\n%s", s, output) + } + } + }) + } +} + +func TestMigratorServiceMonitorWithSkippedChildService(t *testing.T) { + tmpDir := t.TempDir() + + // 1. Valid Service with a pod selector. + validServiceYAML := ` +apiVersion: v1 +kind: Service +metadata: + name: valid-service + namespace: default + labels: + app: my-app +spec: + selector: + app: my-app-pod + ports: + - name: metrics + port: 8080 +` + // 2. Service without a pod selector (external endpoints). + externalServiceYAML := ` +apiVersion: v1 +kind: Service +metadata: + name: external-service + namespace: default + labels: + app: my-app +spec: + ports: + - name: metrics + port: 8080 +` + // 3. ServiceMonitor matching both services via label selector. + smYAML := ` +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: my-servicemonitor + namespace: default +spec: + selector: + matchLabels: + app: my-app + endpoints: + - port: metrics + interval: 30s +` + if err := os.WriteFile(filepath.Join(tmpDir, "valid-svc.yaml"), []byte(validServiceYAML), 0644); err != nil { + t.Fatalf("failed to write valid-svc.yaml: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "external-svc.yaml"), []byte(externalServiceYAML), 0644); err != nil { + t.Fatalf("failed to write external-svc.yaml: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "sm.yaml"), []byte(smYAML), 0644); err != nil { + t.Fatalf("failed to write sm.yaml: %v", err) + } + + migrator := NewMigrator() + migrator.RegisterConverter(&ServiceMonitorConverter{}) + var stdoutBuf, stderrBuf bytes.Buffer + migrator.Stdout = &stdoutBuf + migrator.Stderr = &stderrBuf + + report, err := migrator.Run(tmpDir) + if err != nil { + t.Fatalf("Run failed: %v", err) + } + + // Verify that the ServiceMonitor is tracked as Success when at least one child service converts. + if report.SuccessCount != 1 { + t.Errorf("expected SuccessCount to be 1, got %d", report.SuccessCount) + } + if report.SkippedCount != 0 { + t.Errorf("expected SkippedCount to be 0, got %d", report.SkippedCount) + } +} + +func TestMigratorServiceMonitorAllServicesSkipped(t *testing.T) { + tmpDir := t.TempDir() + + // Service without a pod selector (external endpoints). + externalServiceYAML := ` +apiVersion: v1 +kind: Service +metadata: + name: external-service + namespace: default + labels: + app: my-app +spec: + ports: + - name: metrics + port: 8080 +` + // ServiceMonitor matching only the external service. + smYAML := ` +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: my-external-servicemonitor + namespace: default +spec: + selector: + matchLabels: + app: my-app + endpoints: + - port: metrics +` + if err := os.WriteFile(filepath.Join(tmpDir, "external-svc.yaml"), []byte(externalServiceYAML), 0644); err != nil { + t.Fatalf("failed to write external-svc.yaml: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "sm.yaml"), []byte(smYAML), 0644); err != nil { + t.Fatalf("failed to write sm.yaml: %v", err) + } + + migrator := NewMigrator() + migrator.RegisterConverter(&ServiceMonitorConverter{}) + var stdoutBuf, stderrBuf bytes.Buffer + migrator.Stdout = &stdoutBuf + migrator.Stderr = &stderrBuf + + report, err := migrator.Run(tmpDir) + if err != nil { + t.Fatalf("Run failed: %v", err) + } + + // Verify that the ServiceMonitor is tracked as Skipped when all child services lack selectors. + if report.SkippedCount != 1 { + t.Errorf("expected SkippedCount to be 1, got %d", report.SkippedCount) + } + if report.SuccessCount != 0 { + t.Errorf("expected SuccessCount to be 0, got %d", report.SuccessCount) + } +} diff --git a/pkg/migrate/podmonitor.go b/pkg/migrate/podmonitor.go index 48c34097d1..4cb7bd8389 100644 --- a/pkg/migrate/podmonitor.go +++ b/pkg/migrate/podmonitor.go @@ -117,7 +117,12 @@ func (c *PodMonitorConverter) convertEndpoints( } else if ep.TargetPort != nil { // nolint:staticcheck // Map deprecated TargetPort for backwards compatibility. gmpEp.Port = *ep.TargetPort // nolint:staticcheck // Map deprecated TargetPort for backwards compatibility. } else { - return nil, fmt.Errorf("endpoint [%d]: port or targetPort must be set", i) + convCtx.todos = append(convCtx.todos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Endpoint [%d] does not specify a 'port' or 'targetPort'.", i), + action: "Specify a valid port name or number in 'spec.endpoints[].port'.", + }) + gmpEp.Port = intstr.FromString("TODO_SET_PORT") } // 2. Basic Fields. @@ -126,10 +131,7 @@ func (c *PodMonitorConverter) convertEndpoints( gmpEp.Params = ep.Params // 3. Scrape Intervals & Timeouts. - interval, timeout, err := resolveScrapeIntervalAndTimeout(convCtx.logger, string(ep.Interval), string(ep.ScrapeTimeout)) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } + interval, timeout := convCtx.resolveScrapeIntervalAndTimeout(string(ep.Interval), string(ep.ScrapeTimeout)) gmpEp.Interval = interval gmpEp.Timeout = timeout @@ -137,20 +139,13 @@ func (c *PodMonitorConverter) convertEndpoints( gmpEp.MetricRelabeling = combineAndConvertRelabelings(convCtx.logger, epResults[i].PromotedRules, ep.MetricRelabelConfigs) // Proxy Settings. - proxyURL, err := convertProxyURL(ep.ProxyURL) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } - gmpEp.ProxyURL = proxyURL + gmpEp.ProxyURL = convCtx.convertProxyURL(ep.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. - 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, fmt.Errorf("endpoint [%d]: %w", i, err) - } + convCtx.applyAuthAndTLS(&gmpEp, ep.BasicAuth, ep.OAuth2, ep.TLSConfig, ep.Authorization, ep.BearerTokenSecret) // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. // 5. Warnings for Unsupported Fields in Endpoint. warnUnsupportedEndpointFields(convCtx.logger, ep.FollowRedirects, ep.EnableHttp2, ep.HonorLabels, ep.HonorTimestamps, ep.TrackTimestampsStaleness, i) @@ -167,6 +162,7 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, cache: cache, sourceNamespace: pm.Namespace, targetNamespace: targetNamespace, + isClusterScoped: isCluster, } var relabelConfigs [][]pomonitoringv1.RelabelConfig for _, ep := range pm.Spec.PodMetricsEndpoints { @@ -182,15 +178,25 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, } 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, err - } + mergedSelector := convCtx.mergeLabelSelector(pm.Spec.Selector, rules.ResourceCombined.MatchLabels, rules.ResourceCombined.MatchExpressions) + var todos []todoItem + todos = append(todos, rules.ResourceCombined.Todos...) + if len(mergedSelector.MatchLabels) == 0 && len(mergedSelector.MatchExpressions) == 0 { if isCluster { logger.Warn("Resulting ClusterPodMonitoring selector is empty. It will select and scrape all pods across all namespaces. Verify if this is intended.") + todos = append(todos, todoItem{ + category: "WARNING", + reason: "Resulting ClusterPodMonitoring selector is empty and matches all pods across all namespaces.", + action: "Define explicit 'matchLabels' in 'spec.selector'.", + }) } else { logger.Warn("Resulting PodMonitoring selector is empty. It will select and scrape all pods in this namespace. Verify if this is intended.") + todos = append(todos, todoItem{ + category: "WARNING", + reason: "Resulting PodMonitoring selector is empty and matches all pods in this namespace.", + action: "Define explicit 'matchLabels' in 'spec.selector'.", + }) } } @@ -217,6 +223,7 @@ func (c *PodMonitorConverter) convertMonitorSpec(pm *pomonitoringv1.PodMonitor, filterRunning: filterRunning, limits: limits, generatedSecrets: convCtx.getGeneratedSecrets(), + todos: append(todos, convCtx.todos...), }, nil } diff --git a/pkg/migrate/podmonitor_test.go b/pkg/migrate/podmonitor_test.go index 2a9f0ba12b..59ae115df7 100644 --- a/pkg/migrate/podmonitor_test.go +++ b/pkg/migrate/podmonitor_test.go @@ -83,6 +83,75 @@ func TestPodMonitorConversion(t *testing.T) { }, }, }, + { + name: "Cluster-Scoped with Secret References", + setupCache: func(cache *ResourceCache) error { + return addSecretToCache(cache, "monitoring-ns", "auth-secret", "user", "admin", true) + }, + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-auth-monitor", + Namespace: "monitoring-ns", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + NamespaceSelector: pomonitoringv1.NamespaceSelector{ + Any: true, + }, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "cluster-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "metrics", + BasicAuth: &pomonitoringv1.BasicAuth{ + Username: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "auth-secret"}, Key: "user"}, + Password: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "auth-secret"}, Key: "pass"}, + }, + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.ClusterPodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindClusterPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-auth-monitor", + }, + Spec: monitoringv1.ClusterPodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "cluster-app", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + BasicAuth: &monitoringv1.BasicAuth{ + Username: "admin", + Password: &monitoringv1.SecretSelector{ + Secret: &monitoringv1.SecretKeySelector{ + Name: "auth-secret", + Key: "pass", + Namespace: "monitoring-ns", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, { name: "Multi-Namespace Split", input: &pomonitoringv1.PodMonitor{ @@ -414,6 +483,9 @@ func TestPodMonitorConversion(t *testing.T) { Namespace: "frontend", }, Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "label-app"}, + }, JobLabel: "app-name", PodTargetLabels: []string{"env", "instance", "version"}, PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ @@ -434,6 +506,9 @@ func TestPodMonitorConversion(t *testing.T) { Namespace: "frontend", }, Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "label-app"}, + }, Endpoints: []monitoringv1.ScrapeEndpoint{ { Port: intstr.FromString("metrics"), @@ -1199,10 +1274,17 @@ func TestPodMonitorConversion(t *testing.T) { }, expected: []runtime.Object{ &monitoringv1.PodMonitoring{ - TypeMeta: BuildTypeMeta(KindPodMonitoring), - ObjectMeta: metav1.ObjectMeta{Name: "annotation-keep-monitor", Namespace: "default"}, + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "annotation-keep-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[WARNING] Dropped target filtering rule ('keep' on '__meta_kubernetes_pod_annotation_prometheus_io_scrape'). ACTION: Add equivalent pod label selector in 'spec.selector.matchLabels'.", + "gmp.googleapis.com/todo-2": "[WARNING] Resulting PodMonitoring selector is empty and matches all pods in this namespace. ACTION: Define explicit 'matchLabels' in 'spec.selector'.", + }, + }, Spec: monitoringv1.PodMonitoringSpec{ - Selector: metav1.LabelSelector{}, // Remains empty, selecting all pods. + Selector: metav1.LabelSelector{}, Endpoints: []monitoringv1.ScrapeEndpoint{ { Port: intstr.FromString("metrics"), @@ -1605,10 +1687,34 @@ func TestPodMonitorConversion(t *testing.T) { }, }, }, - wantErr: "selector conflict: label \"app\" has conflicting values \"frontend\" (base selector) and \"backend\" (relabeling rule)", + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "conflict-selector-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Selector conflict: label \"app\" has conflicting values \"frontend\" (from base selector) and \"backend\" (from relabeling rule). ACTION: Reconcile 'spec.selector.matchLabels' for label \"app\" with the intended target pods.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "frontend", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + }, + }, + }, + }, + }, }, { - name: "Pre-Scrape Relabelings: conflicting keep rules on same pod label return error", + name: "Pre-Scrape Relabelings: conflicting keep rules on same pod label attaches TODO annotation", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -1641,10 +1747,35 @@ func TestPodMonitorConversion(t *testing.T) { }, }, }, - wantErr: "conflicting keep rules for label \"env\": cannot require both \"production\" and \"staging\" simultaneously", + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "conflicting-keep-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Conflicting relabeling keep rules for label \"env\": cannot require both \"production\" and \"staging\" simultaneously. ACTION: Define the intended label value for \"env\" in 'spec.selector.matchLabels'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + "env": "production", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + }, + }, + }, + }, + }, }, { - name: "BearerTokenSecret with empty Name returns validation error", + name: "BearerTokenSecret with empty Name generates draft with TODO annotation", input: &pomonitoringv1.PodMonitor{ TypeMeta: metav1.TypeMeta{ APIVersion: "monitoring.coreos.com/v1", @@ -1666,7 +1797,90 @@ func TestPodMonitorConversion(t *testing.T) { }, }, }, - wantErr: "bearerTokenSecret: secret reference has an empty name for key \"token\"", + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "bearer-token-secret-err", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Referenced Secret has an empty name for key \"token\". ACTION: Specify a valid Secret name in the configuration.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "frontend", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("metrics"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + Authorization: &monitoringv1.Auth{ + Credentials: &monitoringv1.SecretSelector{ + Secret: &monitoringv1.SecretKeySelector{ + Name: "TODO_SET_SECRET_NAME", + Key: "token", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "PodMetricsEndpoint missing port and targetPort generates draft with TODO", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-port-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "frontend"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Path: "/metrics", + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-port-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Endpoint [0] does not specify a 'port' or 'targetPort'. ACTION: Specify a valid port name or number in 'spec.endpoints[].port'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "frontend", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_SET_PORT"), + Path: "/metrics", + Interval: "30s", + }, + }, + }, + }, + }, }, { name: "Pre-Scrape Relabelings: drop action rule on pod label falls through to metricRelabelings", @@ -2232,6 +2446,287 @@ func TestPodMonitorConversion(t *testing.T) { "Endpoint-level configuration conflict detected", }, }, + { + name: "PodMonitor with dropped annotation relabeling attaches TODO annotation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "annotated-relabel-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "annotated-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "web", + RelabelConfigs: []pomonitoringv1.RelabelConfig{ + { + Action: "keep", + SourceLabels: []pomonitoringv1.LabelName{"__meta_kubernetes_pod_annotation_prometheus_io_scrape"}, + Regex: "true", + }, + }, + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "annotated-relabel-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[WARNING] Dropped target filtering rule ('keep' on '__meta_kubernetes_pod_annotation_prometheus_io_scrape'). ACTION: Add equivalent pod label selector in 'spec.selector.matchLabels'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "annotated-app", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("web"), + Interval: "30s", + }, + }, + }, + }, + }, + }, + { + name: "PodMonitor with dropped node metadata relabeling attaches TODO annotation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "node-keep-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "node-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "web", + RelabelConfigs: []pomonitoringv1.RelabelConfig{ + { + Action: "keep", + SourceLabels: []pomonitoringv1.LabelName{"__meta_kubernetes_node_label_topology_kubernetes_io_zone"}, + Regex: "us-central1-a", + }, + }, + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "node-keep-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[WARNING] Dropped target filtering rule ('keep' on '__meta_kubernetes_node_label_topology_kubernetes_io_zone'). ACTION: Add equivalent pod label selector in 'spec.selector.matchLabels'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "node-app", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("web"), + Interval: "30s", + }, + }, + }, + }, + }, + }, + { + name: "PodMonitor with empty selector attaches TODO annotation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-selector-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{}, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "web", + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-selector-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[WARNING] Resulting PodMonitoring selector is empty and matches all pods in this namespace. ACTION: Define explicit 'matchLabels' in 'spec.selector'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{}, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("web"), + Interval: "30s", + }, + }, + }, + }, + }, + }, + { + name: "PodMonitor with proxyUrl containing password attaches TODO annotation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-pass-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "proxy-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "web", + ProxyURL: ptrTo("http://user:secret123@proxy.example.com:8080"), + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-pass-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Proxy URL contains embedded plaintext credentials. Credentials were removed. ACTION: Configure proxy authentication via Kubernetes Secret or proxy server configuration.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "proxy-app", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("web"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + ProxyConfig: monitoringv1.ProxyConfig{ + ProxyURL: "http://proxy.example.com:8080", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "PodMonitor with missing BasicAuth secret username injects placeholder and TODO annotation", + input: &pomonitoringv1.PodMonitor{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.coreos.com/v1", + Kind: KindPodMonitor, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-secret-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.PodMonitorSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "missing-app"}, + }, + PodMetricsEndpoints: []pomonitoringv1.PodMetricsEndpoint{ + { + Port: "web", + BasicAuth: &pomonitoringv1.BasicAuth{ + Username: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "missing-auth"}, Key: "user"}, + Password: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "missing-auth"}, Key: "pass"}, + }, + }, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "monitoring.googleapis.com/v1", + Kind: KindPodMonitoring, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "missing-secret-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Referenced SECRET \"missing-auth\" for key \"user\" was not found in migration inputs. ACTION: Verify SECRET \"missing-auth\" exists in namespace \"default\" or provide it in migration inputs.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "missing-app", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("web"), + Interval: "30s", + HTTPClientConfig: monitoringv1.HTTPClientConfig{ + BasicAuth: &monitoringv1.BasicAuth{ + Username: "TODO_SET_USER_FROM_SECRET_MISSING-AUTH", + Password: &monitoringv1.SecretSelector{ + Secret: &monitoringv1.SecretKeySelector{ + Name: "missing-auth", + Key: "pass", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, } converter := &PodMonitorConverter{} diff --git a/pkg/migrate/servicemonitor.go b/pkg/migrate/servicemonitor.go index c6431a1fa4..3e18f52784 100644 --- a/pkg/migrate/servicemonitor.go +++ b/pkg/migrate/servicemonitor.go @@ -41,6 +41,7 @@ type serviceGroup struct { portMap map[string]intstr.IntOrString targetLabels map[string]string services []*corev1.Service + todos []todoItem } // namespaces returns a sorted slice of unique namespaces where Services in this group reside. @@ -96,6 +97,11 @@ func (c *ServiceMonitorConverter) Convert(_ context.Context, logger *slog.Logger var outputs []*unstructured.Unstructured outputs = append(outputs, clusterPodMonitorings...) outputs = append(outputs, generatedSecrets...) + if len(outputs) == 0 { + logger.Info("All matched Services lack pod selectors (target external endpoints). GMP Managed Collection only supports in-cluster Pods. Skipping resource.", + slog.String("migration_status", "skipped"), + ) + } return outputs, nil } @@ -117,6 +123,11 @@ func (c *ServiceMonitorConverter) Convert(_ context.Context, logger *slog.Logger var outputs []*unstructured.Unstructured outputs = append(outputs, podMonitorings...) outputs = append(outputs, generatedSecrets...) + if len(outputs) == 0 { + logger.Info("All matched Services lack pod selectors (target external endpoints). GMP Managed Collection only supports in-cluster Pods. Skipping resource.", + slog.String("migration_status", "skipped"), + ) + } return outputs, nil } @@ -131,13 +142,16 @@ func (c *ServiceMonitorConverter) convertToMonitoringResources( if err != nil { return nil, nil, fmt.Errorf("failed to search for Services matching selector: %w", err) } + if len(groups) == 0 { + return nil, nil, nil + } if len(groups) > 1 { targetKind := "PodMonitoring" if isClusterScoped { targetKind = "ClusterPodMonitoring" } - logger.Warn(fmt.Sprintf("Services matched by selector have conflicts (different selectors, port mappings, or labels). Splitting into multiple %s resources.", targetKind), + logger.Info(fmt.Sprintf("Services matched by selector have conflicts (different selectors, port mappings, or labels). Splitting into multiple %s resources.", targetKind), slog.Int("total_groups", len(groups)), slog.String("servicemonitor", sm.Name)) } @@ -208,16 +222,58 @@ func (c *ServiceMonitorConverter) findAndGroupServices( return nil, fmt.Errorf("failed to search for Services matching selector: %w", err) } if len(svcs) == 0 { - return nil, errors.New("corresponding Kubernetes Service was not found. Selector and port mappings cannot be resolved") + logger.Warn("Corresponding Kubernetes Service was not found. Emitting draft PodMonitoring with placeholder selector and ports.", + slog.String("servicemonitor", sm.Name)) + portMap := make(map[string]intstr.IntOrString) + for _, ep := range sm.Spec.Endpoints { + k := endpointPortKey(ep) + if k == "" { + k = "TODO_SET_PORT" + } + portMap[k] = intstr.FromString("TODO_RESOLVE_PORT") + } + var dummySvcs []*corev1.Service + if len(targetNamespaces) == 0 { + dummySvcs = []*corev1.Service{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: sm.Name, + Namespace: sm.Namespace, + }, + }, + } + } else { + for _, ns := range targetNamespaces { + dummySvcs = append(dummySvcs, &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: sm.Name, + Namespace: ns, + }, + }) + } + } + return []*serviceGroup{ + { + selector: map[string]string{ + "app": "TODO_SET_POD_SELECTOR", + }, + portMap: portMap, + services: dummySvcs, + todos: []todoItem{ + { + category: "ERROR", + reason: "Corresponding Kubernetes Service was not found. Selector and port mappings could not be resolved.", + action: "Define target pod selector in 'spec.selector.matchLabels' and verify endpoint ports.", + }, + }, + }, + }, nil } groups, err := groupServices(logger, sm.Spec.TargetLabels, sm, svcs) if err != nil { return nil, fmt.Errorf("failed to group Services: %w", err) } - if len(groups) == 0 { - return nil, errors.New("no valid Kubernetes Service groups found. Selector and port mappings cannot be resolved") - } return groups, nil } @@ -243,6 +299,7 @@ func (c *ServiceMonitorConverter) buildSpecForGroup( targetNamespace: sm.Namespace, isClusterScoped: isClusterScoped, } + convCtx.todos = append(convCtx.todos, group.todos...) // Extract pre-scrape relabelings. var relabelConfigs [][]pomonitoringv1.RelabelConfig @@ -276,15 +333,25 @@ func (c *ServiceMonitorConverter) buildSpecForGroup( mergedFromPod := mergeFromPod(logger, convertTargetLabels(logger, sm.Spec.PodTargetLabels, "", "Pod"), rules.ResourceCombined.FromPod) baseSelector := metav1.LabelSelector{MatchLabels: group.selector} - mergedSelector, err := mergeLabelSelector(baseSelector, rules.ResourceCombined.MatchLabels, rules.ResourceCombined.MatchExpressions) - if err != nil { - return nil, err - } + mergedSelector := convCtx.mergeLabelSelector(baseSelector, rules.ResourceCombined.MatchLabels, rules.ResourceCombined.MatchExpressions) + var todos []todoItem + todos = append(todos, rules.ResourceCombined.Todos...) + if len(mergedSelector.MatchLabels) == 0 && len(mergedSelector.MatchExpressions) == 0 { if isClusterScoped { logger.Warn("Resulting ClusterPodMonitoring selector is empty. It will select and scrape all pods across all namespaces. Verify if this is intended.") + todos = append(todos, todoItem{ + category: "WARNING", + reason: "Resulting ClusterPodMonitoring selector is empty and matches all pods across all namespaces.", + action: "Define explicit 'matchLabels' in 'spec.selector'.", + }) } else { logger.Warn("Resulting PodMonitoring selector is empty. It will select and scrape all pods in this namespace. Verify if this is intended.") + todos = append(todos, todoItem{ + category: "WARNING", + reason: "Resulting PodMonitoring selector is empty and matches all pods in this namespace.", + action: "Define explicit 'matchLabels' in 'spec.selector'.", + }) } } @@ -311,6 +378,7 @@ func (c *ServiceMonitorConverter) buildSpecForGroup( filterRunning: filterRunning, limits: limits, generatedSecrets: convCtx.getGeneratedSecrets(), + todos: append(todos, convCtx.todos...), }, nil } @@ -331,9 +399,12 @@ func (c *ServiceMonitorConverter) convertEndpointsForGroup( // 1. Port mapping (Use pre-resolved port from group). portKey := endpointPortKey(ep) + if portKey == "" { + portKey = "TODO_SET_PORT" + } resolvedPort, exists := group.portMap[portKey] if !exists { - return nil, fmt.Errorf("endpoint [%d]: port %q was not resolved for this group", i, portKey) + resolvedPort = intstr.FromString("TODO_RESOLVE_PORT") } gmpEp.Port = resolvedPort @@ -343,10 +414,7 @@ func (c *ServiceMonitorConverter) convertEndpointsForGroup( gmpEp.Params = ep.Params // 3. Scrape Intervals & Timeouts. - interval, timeout, err := resolveScrapeIntervalAndTimeout(convCtx.logger, string(ep.Interval), string(ep.ScrapeTimeout)) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } + interval, timeout := convCtx.resolveScrapeIntervalAndTimeout(string(ep.Interval), string(ep.ScrapeTimeout)) gmpEp.Interval = interval gmpEp.Timeout = timeout @@ -354,11 +422,7 @@ func (c *ServiceMonitorConverter) convertEndpointsForGroup( gmpEp.MetricRelabeling = combineAndConvertRelabelings(convCtx.logger, epResults[i].PromotedRules, ep.MetricRelabelConfigs) // Proxy Settings. - proxyURL, err := convertProxyURL(ep.ProxyURL) - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } - gmpEp.ProxyURL = proxyURL + gmpEp.ProxyURL = convCtx.convertProxyURL(ep.ProxyURL) // Auth & TLS mappings. var safeTLS *pomonitoringv1.SafeTLSConfig @@ -382,10 +446,7 @@ func (c *ServiceMonitorConverter) convertEndpointsForGroup( // nolint:staticcheck // Map deprecated BearerTokenSecret for backwards compatibility. bearerTokenSecret = *ep.BearerTokenSecret } - err = convCtx.applyAuthAndTLS(&gmpEp, ep.BasicAuth, ep.OAuth2, safeTLS, ep.Authorization, bearerTokenSecret) // nolint:staticcheck - if err != nil { - return nil, fmt.Errorf("endpoint [%d]: %w", i, err) - } + convCtx.applyAuthAndTLS(&gmpEp, ep.BasicAuth, ep.OAuth2, safeTLS, ep.Authorization, bearerTokenSecret) // nolint:staticcheck // Warnings for Unsupported Fields. warnUnsupportedEndpointFields(convCtx.logger, ep.FollowRedirects, ep.EnableHttp2, ep.HonorLabels, ep.HonorTimestamps, ep.TrackTimestampsStaleness, i) @@ -409,21 +470,32 @@ func groupServices( // 1. Extract and validate selector. selectorString := svc.Spec.Selector if len(selectorString) == 0 { - return nil, fmt.Errorf("service %q has no selector (targets external or static endpoints). GMP Managed Collection only supports scraping in-cluster Pod targets", svc.GetName()) + logger.Info("Service targets external endpoints without a pod selector. GMP Managed Collection only supports in-cluster Pods. Skipping resource.", + slog.String("service", svc.GetName()), + ) + continue } // 2. Resolve ports for this Service. + var groupTodos []todoItem portMap := make(map[string]intstr.IntOrString) for i, ep := range sm.Spec.Endpoints { portKey := endpointPortKey(ep) if portKey == "" { - return nil, fmt.Errorf("endpoint [%d]: port or targetPort must be set", i) - } - resolvedPort, err := resolveServicePort(logger, svc, portKey) - if err != nil { - return nil, fmt.Errorf("service %q: failed to resolve port %q: %w", svc.Name, portKey, err) + portKey = "TODO_SET_PORT" + groupTodos = append(groupTodos, todoItem{ + category: "ERROR", + reason: fmt.Sprintf("Endpoint [%d] does not specify a 'port' or 'targetPort'.", i), + action: "Specify a valid port name or number in 'spec.endpoints[].port'.", + }) + portMap[portKey] = intstr.FromString("TODO_SET_PORT") + } else { + resolvedPort, todo := resolveServicePort(logger, svc, portKey) + if todo != nil { + groupTodos = append(groupTodos, *todo) + } + portMap[portKey] = resolvedPort } - portMap[portKey] = resolvedPort } // 3. Resolve target labels for this Service. @@ -463,12 +535,14 @@ func groupServices( // Merge complementary mappings. maps.Copy(matchedGroup.portMap, portMap) maps.Copy(matchedGroup.targetLabels, resolvedLabels) + matchedGroup.todos = append(matchedGroup.todos, groupTodos...) } else { groups = append(groups, &serviceGroup{ selector: svc.Spec.Selector, portMap: portMap, targetLabels: resolvedLabels, services: []*corev1.Service{svc}, + todos: groupTodos, }) } } diff --git a/pkg/migrate/servicemonitor_test.go b/pkg/migrate/servicemonitor_test.go index 9f53c5f734..449ca5c0a4 100644 --- a/pkg/migrate/servicemonitor_test.go +++ b/pkg/migrate/servicemonitor_test.go @@ -672,7 +672,144 @@ func TestServiceMonitorConverter_Convert(t *testing.T) { }, }, }, - wantErr: true, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "my-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Corresponding Kubernetes Service was not found. Selector and port mappings could not be resolved. ACTION: Define target pod selector in 'spec.selector.matchLabels' and verify endpoint ports.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "TODO_SET_POD_SELECTOR", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_RESOLVE_PORT"), + Interval: "30s", + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Missing backing Service with multiple target namespaces", + setupCache: func(_ *ResourceCache) error { return nil }, + inputSM: &pomonitoringv1.ServiceMonitor{ + TypeMeta: metav1.TypeMeta{APIVersion: "monitoring.coreos.com/v1", Kind: "ServiceMonitor"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.ServiceMonitorSpec{ + NamespaceSelector: pomonitoringv1.NamespaceSelector{ + MatchNames: []string{"ns-1", "ns-2"}, + }, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Endpoints: []pomonitoringv1.Endpoint{ + {Port: "web"}, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "my-monitor", + Namespace: "ns-1", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Corresponding Kubernetes Service was not found. Selector and port mappings could not be resolved. ACTION: Define target pod selector in 'spec.selector.matchLabels' and verify endpoint ports.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "TODO_SET_POD_SELECTOR", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_RESOLVE_PORT"), + Interval: "30s", + }, + }, + }, + }, + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "my-monitor", + Namespace: "ns-2", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Corresponding Kubernetes Service was not found. Selector and port mappings could not be resolved. ACTION: Define target pod selector in 'spec.selector.matchLabels' and verify endpoint ports.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "TODO_SET_POD_SELECTOR", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_RESOLVE_PORT"), + Interval: "30s", + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Missing backing Service with cluster scoping", + setupCache: func(_ *ResourceCache) error { return nil }, + inputSM: &pomonitoringv1.ServiceMonitor{ + TypeMeta: metav1.TypeMeta{APIVersion: "monitoring.coreos.com/v1", Kind: "ServiceMonitor"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster-monitor", + Namespace: "default", + }, + Spec: pomonitoringv1.ServiceMonitorSpec{ + NamespaceSelector: pomonitoringv1.NamespaceSelector{Any: true}, + Selector: metav1.LabelSelector{MatchLabels: map[string]string{"app": "foo"}}, + Endpoints: []pomonitoringv1.Endpoint{ + {Port: "web"}, + }, + }, + }, + expected: []runtime.Object{ + &monitoringv1.ClusterPodMonitoring{ + TypeMeta: BuildTypeMeta(KindClusterPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster-monitor", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Corresponding Kubernetes Service was not found. Selector and port mappings could not be resolved. ACTION: Define target pod selector in 'spec.selector.matchLabels' and verify endpoint ports.", + }, + }, + Spec: monitoringv1.ClusterPodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "TODO_SET_POD_SELECTOR", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_RESOLVE_PORT"), + Interval: "30s", + }, + }, + }, + }, + }, + wantErr: false, }, { name: "Backing Service has no selector", @@ -698,7 +835,8 @@ func TestServiceMonitorConverter_Convert(t *testing.T) { }, }, }, - wantErr: true, + expected: nil, + wantErr: false, }, { name: "Endpoint missing port and targetPort", @@ -724,7 +862,33 @@ func TestServiceMonitorConverter_Convert(t *testing.T) { }, }, }, - wantErr: true, + expected: []runtime.Object{ + &monitoringv1.PodMonitoring{ + TypeMeta: BuildTypeMeta(KindPodMonitoring), + ObjectMeta: metav1.ObjectMeta{ + Name: "my-monitor", + Namespace: "default", + Annotations: map[string]string{ + "gmp.googleapis.com/todo-1": "[ERROR] Endpoint [0] does not specify a 'port' or 'targetPort'. ACTION: Specify a valid port name or number in 'spec.endpoints[].port'.", + }, + }, + Spec: monitoringv1.PodMonitoringSpec{ + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "foo-pod", + }, + }, + Endpoints: []monitoringv1.ScrapeEndpoint{ + { + Port: intstr.FromString("TODO_SET_PORT"), + Path: "/metrics", + Interval: "30s", + }, + }, + }, + }, + }, + wantErr: false, }, { name: "JobLabel conversion from Service", @@ -857,29 +1021,27 @@ func TestServiceMonitorConverter_Convert(t *testing.T) { return } - if tc.expected != nil { - if len(outputs) != len(tc.expected) { - t.Fatalf("expected %d outputs, got %d", len(tc.expected), len(outputs)) + if len(outputs) != len(tc.expected) { + t.Fatalf("expected %d outputs, got %d", len(tc.expected), len(outputs)) + } + for i := range tc.expected { + var gotObj runtime.Object + switch tc.expected[i].(type) { + case *monitoringv1.PodMonitoring: + gotObj = &monitoringv1.PodMonitoring{} + case *monitoringv1.ClusterPodMonitoring: + gotObj = &monitoringv1.ClusterPodMonitoring{} + default: + t.Fatalf("expected object at index %d must be a pointer to a recognized monitoring type, got %T", i, tc.expected[i]) } - for i := range tc.expected { - var gotObj runtime.Object - switch tc.expected[i].(type) { - case *monitoringv1.PodMonitoring: - gotObj = &monitoringv1.PodMonitoring{} - case *monitoringv1.ClusterPodMonitoring: - gotObj = &monitoringv1.ClusterPodMonitoring{} - default: - t.Fatalf("expected object at index %d must be a pointer to a recognized monitoring type, got %T", i, tc.expected[i]) - } - err := runtime.DefaultUnstructuredConverter.FromUnstructured(outputs[i].Object, gotObj) - if err != nil { - t.Fatalf("failed to convert actual to struct: %v", err) - } + err := runtime.DefaultUnstructuredConverter.FromUnstructured(outputs[i].Object, gotObj) + if err != nil { + t.Fatalf("failed to convert actual to struct: %v", err) + } - if diff := cmp.Diff(tc.expected[i], gotObj); diff != "" { - t.Errorf("mismatch at index %d (-want +got):\n%s", i, diff) - } + if diff := cmp.Diff(tc.expected[i], gotObj); diff != "" { + t.Errorf("mismatch at index %d (-want +got):\n%s", i, diff) } } })