✨ Undeprecate plugin optionals - #794
Merged
Merged
Conversation
There was a deprecated transform flag plugin optionals allowing pass args to plugins, this is now needed for e.g. PVC mapping or more powerful plugins. This PR 1. un-deprecates this flag and 2. adds its full support for multistage transformations including instructions file update. Fixes: migtools#791 Signed-off-by: Marek Aufart <maufart@redhat.com>
📝 WalkthroughWalkthroughThe transform command now supports repeatable stage-specific optional flags from CLI arguments or instructions files. YAML stages can include optional configurations. The orchestrator resolves stage-specific flags with global fallback, and the E2E runner forwards these arguments. ChangesStage-specific transform optionals
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TransformCommand
participant InstructionsFile
participant Orchestrator
participant TransformRunner
TransformCommand->>InstructionsFile: load stage names and optionals
TransformCommand->>Orchestrator: pass StageOptionalFlags
Orchestrator->>TransformRunner: run stage with resolved flags
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@cmd/transform/transform.go`:
- Around line 362-366: Update the stage-optionals parsing around json.Unmarshal
so the decoded value must be a JSON object and reject null before assigning
result[stageName]. Preserve the existing invalid-JSON error and ensure null
produces an error instead of an empty stage map, allowing resolveOptionalFlags
to retain global-flag fallback behavior.
- Line 164: Change the stage-optionals flag registration in the command’s option
setup from StringSliceVar to StringArrayVar so each repeated StageName=JSON
argument remains intact, including JSON containing commas. Add a command-level
regression test covering a multi-field JSON stage optional and verify
parseStageOptionals receives and parses it as one value.
In `@internal/transform/instructions.go`:
- Around line 182-186: Reject case-insensitive duplicate optional keys before
normalization instead of allowing map iteration to overwrite values: update the
optional-key handling around the normalized map construction in
internal/transform/instructions.go (lines 182-186) to validate and return an
error on collisions, and add the same validation before optionalFlagsToLower in
cmd/transform/transform.go (lines 362-366). Add regression tests covering both
input paths and preserve the non-destructive export → transform → apply
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f06d16e1-e90b-4970-80fb-1645877f274e
📒 Files selected for processing (8)
cmd/transform/optionals/optionals.gocmd/transform/transform.gocmd/transform/transform_test.goe2e-tests/framework/crane.gointernal/transform/instructions.gointernal/transform/instructions_test.gointernal/transform/orchestrator.gointernal/transform/orchestrator_test.go
Signed-off-by: Marek Aufart <maufart@redhat.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/transform/instructions.go (1)
182-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse one shared optional-key normalizer.
StageOptionalsrepeats the lowercasing and collision checks already used byoptionalFlagsToLowerCheckedincmd/transform/transform.go. These input paths can diverge when the normalization policy changes. Move the shared utility tocrane-liband call it from both paths.As per coding guidelines: “Place shared utilities in
crane-librather than duplicating them.”🤖 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 `@internal/transform/instructions.go` around lines 182 - 189, Move the optional-key normalization and case-insensitive collision validation from the StageOptionals logic into a shared crane-lib utility, preserving its error behavior and normalized map output. Update both StageOptionals and optionalFlagsToLowerChecked in cmd/transform/transform.go to call that utility, removing their duplicated lowercasing and collision-checking logic.Source: Coding guidelines
cmd/transform/transform_test.go (1)
1214-1231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise flag binding in the comma regression test.
TestParseStageOptionals_MultiFieldJSONpasses an already intact string directly toparseStageOptionals. It would pass even if command flag binding split comma-containing values before parsing. Add a command-level test that supplies--stage-optionalsand verifies that one intact value reaches the parser.As per coding guidelines: “Add tests for new features and regression tests for bug fixes when possible.”
🤖 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 `@cmd/transform/transform_test.go` around lines 1214 - 1231, Extend TestParseStageOptionals_MultiFieldJSON with command-level flag binding: supply the multi-field JSON through --stage-optionals using the project’s command/flag setup, execute parsing, and verify both fields are preserved from one intact value. Do not rely solely on directly calling parseStageOptionals; ensure the test exercises StringArrayVar behavior before reaching that parser.Source: Coding guidelines
🤖 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 `@cmd/transform/transform_test.go`:
- Around line 1214-1231: Extend TestParseStageOptionals_MultiFieldJSON with
command-level flag binding: supply the multi-field JSON through
--stage-optionals using the project’s command/flag setup, execute parsing, and
verify both fields are preserved from one intact value. Do not rely solely on
directly calling parseStageOptionals; ensure the test exercises StringArrayVar
behavior before reaching that parser.
In `@internal/transform/instructions.go`:
- Around line 182-189: Move the optional-key normalization and case-insensitive
collision validation from the StageOptionals logic into a shared crane-lib
utility, preserving its error behavior and normalized map output. Update both
StageOptionals and optionalFlagsToLowerChecked in cmd/transform/transform.go to
call that utility, removing their duplicated lowercasing and collision-checking
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68d98acc-4250-49dd-b8ca-1a83cfc19d91
📒 Files selected for processing (4)
cmd/transform/transform.gocmd/transform/transform_test.gointernal/transform/instructions.gointernal/transform/instructions_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/transform/transform.go
- internal/transform/instructions_test.go
Signed-off-by: Marek Aufart <maufart@redhat.com>
Signed-off-by: Marek Aufart <maufart@redhat.com>
Contributor
Author
|
/rfr |
Contributor
Author
|
[review-docs] |
Contributor
📚 Documentation ReviewAnalyzed PR: #794 Found 2 file(s) that may need updates: 📋 Select files to updateUncheck any files you do not want updated:
💡 Next Steps:
Powered by code-to-docs AI ✨ |
Contributor
Author
|
[update-docs] first read https://gist.github.com/aufi/f7421e1238efb760c412b77e9261bdf1 to understand how this change is expected to be used by end-users |
3 tasks
csoceanu
added a commit
to redhat-community-ai-tools/code-to-docs
that referenced
this pull request
Aug 13, 2026
Bot-posted PR comments on fork PRs contained literal [update-docs] text in the footer, which re-triggered the workflow when the bot user was in the allowed users list — creating ~20 duplicate comments (see migtools/crane#794). Also escapes [review-feature] and [review-docs] in error comments posted when Jira credentials are missing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
[update-docs] Take into account following usage examples for feature in this PR: Transform Optional Flags: Usage ExamplesOptional flags control plugin behavior during Discovering Available Flagscrane transform optionalsOutput lists flags per plugin with descriptions and examples: Global Optional Flags
|
| Mode | Highest priority | Lowest priority |
|---|---|---|
With --instructions-file |
per-stage optionals from file |
--optional-flags CLI |
Without --instructions-file |
--stage-optionals CLI |
--optional-flags CLI |
Per-stage overrides (replaces) the global set for that stage. There is no merging.
Mutual Exclusivity
--instructions-file and --stage-optionals cannot be used together:
# This fails with an error
crane transform \
--instructions-file instructions.yaml \
--stage-optionals 'KubernetesPlugin={"registry-replacement": "docker.io=quay.io"}'Use optionals inside the instructions file instead.
KubernetesPlugin Available Flags
| Flag | Description | Example |
|---|---|---|
registry-replacement |
Registry path swap map | docker.io=quay.io,gcr.io=ghcr.io |
add-annotations |
Annotations to add | key1=val1,key2=val2 |
remove-annotations |
Annotations to remove | annotation1,annotation2 |
strip-default-rbac |
Strip default RBAC (default: true) | true / false |
strip-default-cabundle |
Strip default CA bundle (default: true) | true / false |
disable-whiteout-owned |
Disable whiting out owned resources | true |
extra-whiteouts |
Additional resources to whiteout | Deployment.apps,Route.route.openshift.io |
include-only |
Keep only listed resources | Deployment.apps,Service |
pvc-rename-map |
PVC rename mapping | old-name:new-name |
Contributor
📚 Documentation UpdateUpdated 2 file(s) based on your review selections:
📄 Changes
|
Contributor
|
[update-docs] Take into account following usage examples for feature in this PR: Transform Optional Flags: Usage ExamplesOptional flags control plugin behavior during Discovering Available Flagscrane transform optionalsOutput lists flags per plugin with descriptions and examples: Global Optional Flags
|
| Mode | Highest priority | Lowest priority |
|---|---|---|
With --instructions-file |
per-stage optionals from file |
--optional-flags CLI |
Without --instructions-file |
--stage-optionals CLI |
--optional-flags CLI |
Per-stage overrides (replaces) the global set for that stage. There is no merging.
Mutual Exclusivity
--instructions-file and --stage-optionals cannot be used together:
# This fails with an error
crane transform \
--instructions-file instructions.yaml \
--stage-optionals 'KubernetesPlugin={"registry-replacement": "docker.io=quay.io"}'Use optionals inside the instructions file instead.
KubernetesPlugin Available Flags
| Flag | Description | Example |
|---|---|---|
registry-replacement |
Registry path swap map | docker.io=quay.io,gcr.io=ghcr.io |
add-annotations |
Annotations to add | key1=val1,key2=val2 |
remove-annotations |
Annotations to remove | annotation1,annotation2 |
strip-default-rbac |
Strip default RBAC (default: true) | true / false |
strip-default-cabundle |
Strip default CA bundle (default: true) | true / false |
disable-whiteout-owned |
Disable whiting out owned resources | true |
extra-whiteouts |
Additional resources to whiteout | Deployment.apps,Route.route.openshift.io |
include-only |
Keep only listed resources | Deployment.apps,Service |
pvc-rename-map |
PVC rename mapping | old-name:new-name |
Contributor
📚 Documentation UpdateUpdated 2 file(s) based on your review selections:
📄 Changes
|
| Plugin Type | Recommended Priority | Keywords |
|---|---|---|
| @@ -208,12 +258,6 @@ | ||
| export/ → 10_KubernetesPlugin/ → 20_OpenshiftPlugin/ → 30_ImagestreamPlugin/ → output/ |
-Each stage:
-1. Reads input resources (from export or previous stage)
-2. Applies transformations via plugins
-3. Writes output to its stage directory
-4. Next stage uses this output as input
-
## Workflow Examples
### Example 1: Simple Transform and Apply
@@ -249,59 +293,11 @@
--transform-dir transform \
20_OpenshiftPlugin
-# Create ImageStream transformations
-crane transform \
- --transform-dir transform \
- 30_ImagestreamPlugin
-
# Apply all stages
crane apply --transform-dir transform --output-dir output
-### Example 3: Iterative Development
-```bash
-# Initial transform
-crane transform --export-dir export --transform-dir transform
-# Make manual edits to resources in transform/10_KubernetesPlugin/input/
-# Edit deployment.yaml to add annotations, etc.
-# Try to re-run transform (will fail due to dirty check)
-crane transform --export-dir export --transform-dir transform
-# Error: contains user modifications
-# Force overwrite if needed
-crane transform --export-dir export --transform-dir transform --force
-# Or preserve changes by creating a new stage
-crane transform 20_custom
-```
Migration from JSONPatch Workflow
-### Old Workflow
-text -transform/ -├── namespace/ -│ └── default/ -│ └── deployment/ -│ └── myapp.json # JSONPatch per resource -
-### New Workflow
-```text
-transform/
-└── 10_KubernetesPlugin/
- ├── input/
- │ └── deployment.yaml # Grouped by type
- ├── patches/
- │ └── deployment-myapp-default.yaml
- ├── output/ # Materialized output
- └── kustomization.yaml
-```
Benefits
@@ -311,48 +307,6 @@
4. Better Diff: Deterministic ordering produces stable Git diffs
5. Dirty Check: Prevents accidental overwrites of user modifications
-## Advanced Features
-### Stage Validation
-Validate stage names before applying:
-```go
-import (
- "github.com/konveyor/crane/internal/transform"
-)
-// Validate stage names
-stages, err := transform.DiscoverStages(transformDir)
-if err != nil {
- fmt.Printf("failed to discover stages: %v\n", err)
-}
-for _, stage := range stages {
- if err := transform.ValidateStageName(stage.DirName); err != nil {
-
fmt.Printf("Invalid stage name %s: %v\n", stage.DirName, err) - }
-}
-```
-### Custom Stage Naming
-Generate stage names with priority numbers:
-```go
-import "github.com/konveyor/crane/internal/transform"
-// Generate a stage name with priority
-stageName := transform.GenerateStageName(15, "my-plugin")
-// Returns: "15_my-plugin"
-// Validate a stage name
-err := transform.ValidateStageName("15_my-plugin")
-if err != nil {
- // Handle invalid stage name
-}
-```
Troubleshooting
Issue: Transform fails with "contains user modifications"
@@ -372,40 +326,6 @@
- Check kustomization.yaml syntax
- Verify all resource files exist in input/
- Run
crane apply <stage>to isolate the failing stage
-### Issue: Resources not appearing in output
-Cause: Resources may be whiteout (excluded) by plugins.
-Solution:
-1. Check whiteout-report.yaml in stage directory
-2. Review plugin configuration
-3. Check plugin logs for whiteout decisions
-### Issue: Patches not being applied
-Cause: Patch file or target selector may be incorrect.
-Solution:
-1. Verify patch file exists in patches/
-2. Check target selector matches resource metadata
-3. Review ignored-patches-report.yaml for conflicts
-## Best Practices
-1. Stage Naming: Use descriptive names that indicate the transformation purpose
-
- Good:
10_KubernetesPlugin-base,20_OpenshiftPlugin-routes,30_security-context
- Good:
-
- Bad:
10_stage1,20_stage2
- Bad:
-2. Priority Spacing: Leave gaps (10, 20, 30) to allow insertion of new stages
-3. Version Control: Commit transform directories to Git to track changes
-4. Testing: Always test transformed output before applying to production
-5. Incremental Changes: Use separate stages for different concerns (security, networking, storage)
-6. Documentation: Include README.md in transform directory explaining pipeline purpose
API Reference
@@ -427,35 +347,6 @@
last := transform.GetLastStage(stages)
prev := transform.GetPreviousStage(stages, currentStage)
next := transform.GetNextStage(stages, currentStage)
-// Stage name validation and generation
-err = transform.ValidateStageName("10_KubernetesPlugin")
-stageName := transform.GenerateStageName(10, "kubernetes")
-```
-### Apply Package
-```go
-// Kustomize apply (embedded — no kubectl dependency)
-applier := &apply.KustomizeApplier{
- Log: logger,
- TransformDir: transformDir,
- OutputDir: outputDir,
- SkipClusterScoped: false,
-}
-// Apply a single stage
-err = applier.ApplySingleStage("10_KubernetesPlugin")
-// Apply multiple stages with selector
-selector := transform.StageSelector{
- FromStage: "10_KubernetesPlugin",
- ToStage: "30_ImagestreamPlugin",
-}
-err = applier.ApplyMultiStage(selector)
-// Apply all stages sequentially
-err = applier.ApplyMultiStage(transform.StageSelector{})
## Further Reading
@@ -472,40 +363,12 @@
> **Warning — Namespace renaming:** If you manually rename the namespace in resource files within a custom stage (for example, changing `namespace: old-ns` to `namespace: new-ns`), Crane does not automatically update **ClusterRoleBinding subjects** referencing the old namespace name, or **NetworkPolicy `namespaceSelector`** entries matching the old namespace by label (e.g., `kubernetes.io/metadata.name: old-ns`). These will silently break after migration. Manually update them as well.
-### Example
-
-```bash
-# Create a multi-stage pipeline
-crane transform 10_KubernetesPlugin # Plugin-backed
-crane transform 50_ManualEdits # No matching plugin
-crane transform 90_FinalCleanup # No matching plugin
-```
-
-**What happens:**
-
-1. **10_KubernetesPlugin**: Resources transformed by KubernetesPlugin (removes metadata.uid, etc.)
-2. **50_ManualEdits**: Resources copied unchanged to `transform/50_ManualEdits/input/`
- - No plugins match "ManualEdits"
- - No patches generated
- - You can manually edit resources in this stage
-3. **90_FinalCleanup**: Resources from previous stage copied unchanged
- - User can add manual patches or edits
-
-### Behavior
-
-When stage name doesn't match any plugin:
-- `filterPluginsByStage()` returns empty list `[]`
-- `runner.Run()` called with empty plugin list
-- Resources written unchanged (no transformations)
-- No patches generated
-- **This is intentional** - allows user-controlled stages
-
### Mixed Pipeline Example
```text
export/
└── resources/
- └── deployment.yaml (raw export with uid, resourceVersion, etc.)
+ └── deployment.yaml
↓
transform/10_KubernetesPlugin/ (plugin: removes server-managed fields)
└── input/deployment.yaml (cleaned)
commands/transform.md
View diff
--- a/commands/transform.md
+++ b/commands/transform.md
@@ -188,6 +188,71 @@
- **`resources/<namespace>/`**: Individual resource files organized by namespace for easier review and selective application
- **`resources/_cluster/`**: Cluster-scoped resources (omitted when `--skip-cluster-scoped` is set)
+## Plugin Optional Flags
+
+Plugins may accept optional flags to modify their transformation logic.
+
+### Discovering Available Flags
+
+Run the following command to see which optional fields are accepted by configured plugins:
+
+```bash
+crane transform optionals
+```
+
+### Global Optional Flags
+
+Use `--optional-flags` to pass a JSON object of flags to all plugins across all stages.
+
+```bash
+# Registry replacement for all stages
+crane transform --optional-flags '{"registry-replacement": "docker.io=quay.io"}'
+
+# Multiple flags
+crane transform --optional-flags '{"registry-replacement": "docker.io=quay.io", "add-annotations": "migrated-by=crane"}'
+```
+
+### Per-Stage Optional Flags (CLI)
+
+Use the repeatable `--stage-optionals` flag to set options for specific stages. The stage name refers to the plugin name (e.g., `KubernetesPlugin`), not the directory name (`10_KubernetesPlugin`).
+
+```bash
+# Different registry replacement per stage
+crane transform \
+--stage-optionals 'KubernetesPlugin={"registry-replacement": "docker.io=quay.io"}' \
+--stage-optionals 'OpenshiftPlugin={"registry-replacement": "docker.io=registry.redhat.io"}'
+```
+
+### Per-Stage Optional Flags (Instructions File)
+
+When using an `--instructions-file`, you can define optionals directly within the file.
+
+```yaml
+# instructions.yaml
+stages:
+- name: KubernetesPlugin
+ optionals:
+ registry-replacement: "docker.io=quay.io"
+- name: OpenshiftPlugin
+ optionals:
+ registry-replacement: "docker.io=registry.redhat.io"
+```
+
+```bash
+crane transform --instructions-file instructions.yaml
+```
+
+### Precedence and Constraints
+
+- **Overrides**: Per-stage optionals replace the entire global set for that stage (there is no merging).
+- **Fallback**: Stages without per-stage flags inherit global `--optional-flags`.
+- **Mutual Exclusivity**: `--instructions-file` and `--stage-optionals` cannot be used together.
+
+| Mode | Highest priority | Lowest priority |
+|------|-----------------|-----------------|
+| With `--instructions-file` | per-stage `optionals` from file | `--optional-flags` CLI |
+| Without `--instructions-file` | `--stage-optionals` CLI | `--optional-flags` CLI |
+
## Automatic Stage Creation
When no stages exist in the transform directory, `crane transform` automatically creates stages for **all available plugins** (not just KubernetesPlugin). Plugins are sorted alphabetically and assigned priorities starting at 10, incrementing by 5. Use `--skip-plugins` to exclude specific plugins from this default behavior.
@@ -309,7 +374,7 @@
### kustomization.yaml
-Kustomize configuration that ties everything together:
+Kustomize configuration that ties together:
```yaml
apiVersion: kustomize.config.k8s.io/v1beta1
@@ -326,12 +391,7 @@
- input/ConfigMap__v1_default_nginx-config.yaml
- input/Deployment_apps_v1_default_wordpress.yaml
- input/Service__v1_default_kubernetes.yaml
-
-# Whiteout resources are written to input/ for complete snapshot
-# but excluded from active resources list above:
-# - input/Pod__v1_default_wordpress-74b89cc84c-nm9f8.yaml
-```
-
+```
## Common Workflows
A docs PR has been created: #816
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a deprecated transform flag plugin optionals allowing pass args to plugins (from #545), this is now needed for e.g. PVC mapping for data migration flow support or more powerful plugins. This PR 1. un-deprecates this flag and 2. adds its full support for multistage transformations including instructions file update.
Doc update is not part of this PR as I understood it should be automated in different PR. Examples on use-cases is at https://gist.github.com/aufi/f7421e1238efb760c412b77e9261bdf1, cc @Tamar-Dinavetsky
Fixes: #791
Summary by CodeRabbit
New Features
--stage-optionalsarguments or instructions files.Bug Fixes
Improvements