Embed OpenshiftPlugin as default plugin#6
Conversation
📝 WalkthroughWalkthroughThis pull request adds the OpenShift transform plugin as a built-in plugin. The change adds a module dependency, registers the OpenShift plugin alongside Kubernetes in the plugin helper, and updates test expectations to reflect the expanded built-in plugin set. ChangesOpenShift Plugin Integration
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Test Coverage ReportTotal: 46.5% Per-package coverage
Full function-level detailsPosted by CI |
|
|
||
| // Start with built-in plugins | ||
| unfilteredPlugins = append(unfilteredPlugins, &kubernetes.KubernetesTransformPlugin{}) | ||
| unfilteredPlugins = append(unfilteredPlugins, &openshift.OpenShiftTransformPlugin{Log: logger}) |
There was a problem hiding this comment.
👍 We probably should add the logger to KubernetesPlugin too, commented it on migtools#494
There was a problem hiding this comment.
yes, that would need a change in crane-lib first i think
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e-tests/tests/mta_801_stateful_migration_test.go (1)
16-16: ⚡ Quick winSystematic loss of tier0 stateful migration test coverage.
All five tier0 E2E tests for stateful migration (PVC transfer) are now skipped because the
transfer-pvccommand is disabled inmain.go. This removes test coverage for: basic stateful migration (MTA-801), empty PVC migration (MTA-804), MySQL data integrity validation (MTA-807, BUG#213), MongoDB non-admin migration (MTA-811, BUG#213), and CronJob+PVC migration (MTA-813, BUG#330/migtools#408).Consider creating a tracking issue for re-enabling these tests when
transfer-pvcis restored, and referencing that issue in a comment above each Skip call so future maintainers understand this is temporary rather than permanent test removal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-tests/tests/mta_801_stateful_migration_test.go` at line 16, All five tier0 stateful migration E2E tests were skipped due to disabling the transfer-pvc command; create a tracking issue (e.g., "re-enable transfer-pvc and restore stateful migration E2E tests") and add a brief comment above each Skip(...) call in the affected test files (e.g., Skip in e2e-tests/tests/mta_801_stateful_migration_test.go and the other four stateful migration test files) that references the new issue number and notes this is temporary until transfer-pvc is restored in main.go; do not change the Skip behavior—only add the comment and open the tracking issue so future maintainers know to re-enable the tests when transfer-pvc is reintroduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e-tests/tests/mta_801_stateful_migration_test.go`:
- Line 16: All five tier0 stateful migration E2E tests were skipped due to
disabling the transfer-pvc command; create a tracking issue (e.g., "re-enable
transfer-pvc and restore stateful migration E2E tests") and add a brief comment
above each Skip(...) call in the affected test files (e.g., Skip in
e2e-tests/tests/mta_801_stateful_migration_test.go and the other four stateful
migration test files) that references the new issue number and notes this is
temporary until transfer-pvc is restored in main.go; do not change the Skip
behavior—only add the comment and open the tracking issue so future maintainers
know to re-enable the tests when transfer-pvc is reintroduced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7651ecbd-e03d-4ae1-991a-ab8d017dd934
📒 Files selected for processing (6)
e2e-tests/tests/mta_801_stateful_migration_test.goe2e-tests/tests/mta_804_empty_pvc_migration_test.goe2e-tests/tests/mta_806_pvc_data_integrity_test.goe2e-tests/tests/mta_807_data_validation_test.goe2e-tests/tests/mta_811_mongodb_non_admin_test.goe2e-tests/tests/mta_813_cronJob_PVC_test.go
✅ Files skipped from review due to trivial changes (1)
- e2e-tests/tests/mta_806_pvc_data_integrity_test.go
PR: Embed OpenShift plugin into mta-crane as a built-in
Addresses #455. mta-crane now ships with the OpenShift plugin embedded alongside KubernetesPlugin — no separate binary download needed.
What changed
Added
github.com/migtools/crane-plugin-openshift/openshift v0.1.0as a dependency and registeredOpenShiftTransformPluginas a built-in plugin inplugin_helper.go.Result
$ crane transform list-plugins
Plugin: KubernetesPlugin (version v0.0.10)
Plugin: OpenShiftPlugin (version v0.1.0)
Transform auto-creates two stages and runs them sequentially:
10_KubernetesPlugin— strips server-managed metadata15_OpenShiftPlugin— whiteouts OCP default resourcesTesting
7 mixed resources through the full pipeline (export → transform → apply):
Summary by CodeRabbit
New Features
Tests
Chores