Skip to content

PO to GMP Migration Tool: Add Draft Generation with TODO Annotations for Errors - #2061

Merged
karthunni merged 10 commits into
mainfrom
karthunni/po-migrate-guardrails-todos
Aug 13, 2026
Merged

PO to GMP Migration Tool: Add Draft Generation with TODO Annotations for Errors#2061
karthunni merged 10 commits into
mainfrom
karthunni/po-migrate-guardrails-todos

Conversation

@karthunni

Copy link
Copy Markdown
Collaborator

This PR enhances the Prometheus Operator to GMP migration tool by implementing graceful draft generation for conversion errors instead of aborting the entire migration. The converter produces manifests with TODO annotations, placeholder values, and a safety guardrail label.

  • Missing Secret / ConfigMap Keys & References:
    • Emits TODO_MISSING_KEY_<KEY>_IN_<KIND>_<NAME> and TODO_SET_SECRET_NAME placeholders with [ERROR] TODO annotations instead of returning hard errors.
  • Duration Parsing & Scrape Timeouts:
    • Defaults invalid scrape intervals to "30s" with [ERROR] TODOs to preserve schema validity.
    • Drops invalid scrape timeouts and caps timeouts exceeding scrape intervals with actionable TODOs.
  • Endpoint & Service Port Resolution:
    • Emits port: "TODO_SET_PORT" when an endpoint omits both port and targetPort.
    • Resolves unmapped Service ports to port: "TODO_RESOLVE_PORT_<PORT>" with [WARNING] TODOs.
  • Missing Backing Services for ServiceMonitor:
    • When a ServiceMonitor has no matching backing Service in the inputs, generates a draft PodMonitoring with selector: {app: "TODO_SET_POD_SELECTOR"} and port: "TODO_RESOLVE_PORT".
  • Selector & Keep Rule Conflicts:
    • Preserves base selectors and records actionable TODOs when relabel keep rules conflict with existing match labels.
  • Safety Guardrail Label:
    • Injects gmp.googleapis.com/migration-review-required: "true" into spec.selector.matchLabels whenever [ERROR] TODO items are present, preventing unintentional scraping until user review.

@karthunni karthunni self-assigned this Aug 7, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to inject sequential TODO annotations and safety guardrails (gmp.googleapis.com/migration-review-required: "true") into migrated resources when configuration issues are encountered, and updates the migration report to track these action items. The review feedback highlights several violations of migration rules where the code falls back to default values or placeholders (such as for invalid proxy URLs, invalid scrape intervals, or unresolvable Service ports) instead of returning a fatal error. Malformed configurations that would lead to failed scrapes must result in errors rather than falling back to defaults or logging warnings.

Comment thread pkg/migrate/helpers.go Outdated
Comment thread pkg/migrate/helpers.go
Comment thread pkg/migrate/servicemonitor.go
Comment thread pkg/migrate/podmonitor.go
Comment thread pkg/migrate/servicemonitor.go
@karthunni
karthunni requested a review from dashpole August 7, 2026 21:21

@bwplotka bwplotka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like a reasonable approach if you can make it a pattern for all errors/warnings (e.g. we don't forget about something).

Not sure about blocking scrape logic - it feels odd, but it is one of the least bad options if we assume people or agents will apply without looking

@karthunni
karthunni force-pushed the karthunni/po-migrate-guardrails-todos branch from a88cc48 to 460df7d Compare August 10, 2026 15:38
Comment thread pkg/migrate/servicemonitor.go Outdated
Comment thread pkg/migrate/helpers.go
Comment thread pkg/migrate/migrate.go Outdated
Comment thread pkg/migrate/servicemonitor_test.go
Comment thread pkg/migrate/helpers.go Outdated
@karthunni

Copy link
Copy Markdown
Collaborator Author

Looks like a reasonable approach if you can make it a pattern for all errors/warnings (e.g. we don't forget about something).

Not sure about blocking scrape logic - it feels odd, but it is one of the least bad options if we assume people or agents will apply without looking

Reconsidered potential options and decided to take the following approach:

On a default CLI call (i.e no flags), we output the manifests without TODO items, while noting in the stderr output summary: (such that users can safely blindly apply)

X 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.

When the --all flag is added, all manifests are written to stdout including the ones with TODOs (still in the annotations but no longer blocking scraping logic with the matchLabels), and the output summary states: (such that an explicit call is needed to output "dangerous" CRs)

X 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.

@karthunni
karthunni marked this pull request as ready for review August 12, 2026 18:17
@karthunni
karthunni force-pushed the karthunni/po-migrate-guardrails-todos branch from 6026b22 to 3c4fdac Compare August 12, 2026 18:52
Comment thread pkg/migrate/podmonitor.go
Comment thread pkg/migrate/servicemonitor.go Outdated
Comment thread pkg/migrate/podmonitor_test.go Outdated
@karthunni
karthunni merged commit 82d36e3 into main Aug 13, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants