Skip to content

fix(mainhandler): re-render persisted schedules from the current template at startup - #447

Merged
matthyx merged 1 commit into
kubescape:mainfrom
blacksoxx:schedule-cronjob-migration
Oct 21, 2026
Merged

matthyx merged 1 commit into
kubescape:mainfrom
blacksoxx:schedule-cronjob-migration

Conversation

@blacksoxx

@blacksoxx blacksoxx commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Overview

A backend schedule CronJob is rendered from the chart's template ConfigMap once, when it is created; updateVulnScanCronJob and updateKubescapeCronJob rewrite only the schedule and the annotations. kubescape/helm-charts#964 moved the templates from an http-request pod posting to operator:4002/v1/triggerAction to a pod that creates an OperatorCommand, and kubescape/helm-charts#971 drops the trigger-port ingress rule, so a schedule created before #964 would keep posting to a port the NetworkPolicy no longer admits (matthyx's P2 on #971).

MigrateScheduleCronJobs runs once at startup, after the main handler is set up. It lists the CronJobs in the operator namespace and, for each whose pods carry the armo.tier label of a schedule (vuln-scan, kubescape-scan, registry-scan) and whose container args still name v1/triggerAction, re-renders the PodSpec from that tier's template ConfigMap with the request volume pointing at the schedule's own ConfigMap, as setCronJobForTriggerRequest does at creation. Name, schedule, time zone, labels, annotations and the request ConfigMap stay as they are. A template that still renders the HTTP call (operator restarted before the chart upgrade) or that is not installed (component disabled) leaves its schedules alone. Every schedule is tried; the first failed update is returned and logged as a warning in main.

How to Test

go test -race ./mainhandler/
  • TestMigrateScheduleCronJobsRerendersTheSchedulesThatStillPostToTheOperator: a fake cluster with the three current templates, one pre-#964 schedule and one current schedule per tier, plus a CronJob without the schedule label; only the three old schedules are updated, each gets the current template's PodSpec with its own request ConfigMap, and its schedule, time zone, labels and annotations are unchanged.
  • TestMigrateScheduleCronJobsLeavesSchedulesAloneWhenTheTemplateCannotHelp: an old schedule stays when its template still posts to the operator and when the template is not installed.
  • TestMigrateScheduleCronJobsReportsTheFirstFailureAfterTryingEverySchedule: a failing update is returned and the other schedule is still migrated.

main.go builds and vets for linux/amd64 (the root package does not build on macOS because of inspektor-gadget's build constraints).

Related issues/PRs:

Summary by CodeRabbit

  • New Features

    • Existing scheduled jobs are automatically updated when the service starts, helping them continue running with current scheduling behavior.
    • Schedule timing and associated configuration are preserved during the update.
  • Bug Fixes

    • Startup now continues even if an individual scheduled job cannot be updated.
    • Updates proceed for other eligible schedules when one job encounters an error.

…late at startup

Signed-off-by: Youssef Omar Bouden <youssef.bouden2002@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 26646d2d-3054-4ad9-a874-61c21a4cdf2d
📥 Commits

Reviewing files that changed from the base of the PR and between c29b7da and 8891d83.

📒 Files selected for processing: 3
  • main.go
  • mainhandler/cronjobmigration.go
  • mainhandler/cronjobmigration_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

At startup, the operator migrates eligible schedule CronJobs using available tier templates before starting the REST API listener. Migration errors are logged as warnings, and startup continues.

Changes: Schedule CronJob Migration

Layer / File(s) Summary
Detect and migrate legacy schedules
mainhandler/cronjobmigration.go, mainhandler/cronjobmigration_test.go
The migration identifies schedules that use the legacy trigger endpoint and updates them from available tier templates. Tests cover template availability, schedule metadata, and update failures.
Run migration during startup
main.go
main calls the migration before starting the REST API listener. Migration errors produce warnings and do not stop startup.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Main
  participant Migration as MigrateScheduleCronJobs
  participant KubernetesApi
  participant Listener as REST API listener
  Main->>Migration: migrate configured namespace schedules
  Migration->>KubernetesApi: list and update CronJobs
  Migration-->>Main: return migration result
  Main->>Listener: start listener
Loading

Possibly related issues

  • Issue 446 in kubescape/operator: The change re-renders persisted schedules that retain the old v1/triggerAction PodSpec.
Merge Risk: ⚪ Minimal · up to 8891d

No identified issue blocks merging the schedule migration after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: re-rendering persisted schedules from the current template during startup.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matthyx matthyx 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.

Approve — no blocking findings in the three-file migration at 8891d833d2f47c993de588b59fc59bdba0a4c0d7, targeting main at c29b7dae93f3c5e44e64b428a4d6162e3b200d7c (PR merge base 6bd65ac2c08716930d00313bcab897ebdde928ab).

The fix is needed: #446 and the upgrade finding on helm-charts#971 describe persisted HTTP schedules. Current main's update handlers still change only schedule/annotations; it has no equivalent startup migration. Replacing template ConfigMaps in merged helm-charts#964 cannot update those persisted CronJobs. This implementation replaces the complete execution PodSpec for all three legacy tiers, retains schedule/time zone/metadata and the schedule's request ConfigMap, skips current jobs and old/missing templates, and continues after individual failures. No dependencies or unrelated changes are introduced.

History: searched operator PRs across open, closed and merged states and related issues using schedule, cronjob, triggerAction, http-request, persisted, migration, MigrateScheduleCronJobs and cronjobmigration, up to 100 results/query. No duplicate or superseding persisted-schedule migration surfaced. The broad http-request query reached the 100-result cap; that cap and search indexing/body coverage limit completeness. #431 concerns listener configuration; #435 supplies the already-merged parent-tracking repair. #411 was explicitly rejected because its allowlist broke intended CLI remediation. That objection does not apply: this PR leaves the HTTP endpoint available. Earlier #964 image/command and tracking objections were addressed before its approval; #447 supplies the subsequent persisted-resource repair requested on #971.

Validation: in a credential-free, network-isolated Bubblewrap sandbox with read-only source/dependencies and disposable writable build cache, go test -race ./mainhandler/ passed; go build -o /tmp/operator . and go vet . ./mainhandler/ passed on linux/amd64 with Go 1.26.9. git diff --check passed. Tests cover three tiers, preservation, compatibility skips and continuation after update failure. Independent code and architecture reviews found no material blocker. No live-cluster upgrade, real scheduler execution, or failing-before regression run was performed. Startup cancellation and retry improvements are optional follow-ups to the documented best-effort migration.

At the final context check there were no existing reviews or inline findings on #447. Integration build and reported security/compliance checks passed; the cross-platform build remained running. Pending CI alone is not a blocker. The chart still needs a released operator containing this fix before removing legacy ingress, as its author already acknowledged on #971.

@matthyx matthyx added the release Create release label Oct 21, 2026
@matthyx
matthyx merged commit 470f33e into kubescape:main Oct 21, 2026
10 checks passed
@matthyx matthyx moved this to To Archive in KS PRs tracking Oct 21, 2026
@blacksoxx

Copy link
Copy Markdown
Contributor Author

Thanks for the review and the merge; the three tiers and the kept request ConfigMap were the parts I most wanted a second pair of eyes on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release Create release

Projects

Status: To Archive

Development

Successfully merging this pull request may close these issues.

Schedules created before helm-charts#964 keep posting to the trigger port after an upgrade

2 participants