Added support to export CRDs#205
Merged
stillalearner merged 4 commits intoApr 1, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds CRD-export support: normalizes include/skip group lists, filters out builtin and subresource API groups, deduplicates plural.group keys from referenced resources, fetches matching CustomResourceDefinitions via a dynamic client, returns synthetic CRD resources for successes and resource-error entries for failed GETs. (36 words) Changes
Sequence DiagramsequenceDiagram
participant Export as ExportOptions.Run()
participant Collector as collectRelatedCRDs
participant Client as dynamicClient
participant Logger as logrus.FieldLogger
participant ErrList as groupResourceError
Export->>Collector: resources, dynamicClient, Logger, include/skip sets
Collector->>Collector: normalize sets\nfilter builtin groups\nskip subresource names\ndedupe plural.group keys
loop for each deduped CRD name
Collector->>Client: GET apiextensions.k8s.io/v1 customresourcedefinitions/<name>
alt GET success
Client-->>Collector: unstructured CRD
Collector->>Export: synthetic groupResource with CRD
else GET failure
Client-->>Collector: error (Forbidden/NotFound/Other)
Collector->>ErrList: create groupResourceError (APIResource name = crdFailureAPIResourceName(<name>))
Collector->>Export: return error entry
end
end
Collector-->>Export: crdResources[], crdErrs[]
Export->>Export: append crdResources to resources\nappend crdErrs to resourceErrs\nwriteResources(...)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/export/export.go`:
- Around line 152-154: collectRelatedCRDs is appending cluster-scoped CRD
objects into resources without ensuring the destination directory exists,
causing writeResources to fail when --cluster-scoped-rbac is off; modify the
export flow so that before appending crdResources (the result of
collectRelatedCRDs) you create the corresponding "resources/<ns>/_cluster"
directory when any CRD with empty Namespace is present (or always create the
_cluster dir when collectRelatedCRDs returns non-empty crdResources), ensuring
the same directory-creation logic used in the code path for
--cluster-scoped-rbac is applied; update the code around the resources =
append(resources, crdResources...) line and/or the collectRelatedCRDs caller to
call the existing directory-creation helper (or create the directory) so
writeResources can successfully write cluster-scoped CRDs.
🪄 Autofix (Beta)
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
Run ID: 90bb5820-debe-4ea7-9d48-5ab013ef78f3
📒 Files selected for processing (3)
cmd/export/crd.gocmd/export/crd_test.gocmd/export/export.go
Contributor
Author
|
To be merged only after #195 |
Contributor
Author
|
Linked to #186 |
aufi
reviewed
Mar 30, 2026
nachandr
reviewed
Mar 30, 2026
…nclude/exclude apigroups by user
stillalearner
force-pushed
the
add_crd_to_export_command
branch
from
a7987ff to
933f8c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/export/crd.go (2)
84-123: Consider sorting CRD names for deterministic output order.Iterating over
seenmap produces non-deterministic order across runs. While functionally correct, this may cause diff noise in exported directories or make debugging harder when comparing exports.♻️ Proposed fix for deterministic iteration
+ import "sort" + if len(seen) == 0 { return nil, nil } + crdNames := make([]string, 0, len(seen)) + for name := range seen { + crdNames = append(crdNames, name) + } + sort.Strings(crdNames) + crdClient := dynamicClient.Resource(crdGVR) out := make([]*groupResource, 0, len(seen)) var outErrs []*groupResourceError - for crdName := range seen { + for _, crdName := range crdNames { obj, err := crdClient.Get(context.Background(), crdName, metav1.GetOptions{})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/export/crd.go` around lines 84 - 123, The loop over the map variable seen yields non-deterministic CRD order; change the iteration to collect keys from seen into a slice, sort that slice (e.g., using sort.Strings), and then iterate over the sorted slice (using crdName from the sorted keys) when calling crdClient.Get and appending to out/outErrs; update any references inside the loop that assume the original map iteration but keep the existing error handling and log messages in the block that uses crdName, crdClient.Get, outErrs, and out.
85-85: Consider accepting acontext.Contextparameter for cancellation support.Using
context.Background()prevents callers from cancelling long-running CRD fetches (e.g., on SIGINT or timeout). If the export operation becomes long-running, this could leave orphaned requests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/export/crd.go` at line 85, The code uses context.Background() when calling crdClient.Get, which prevents cancellation; change the enclosing function to accept a context.Context parameter (replace context.Background() with the passed ctx) and propagate that ctx through to crdClient.Get(ctx, crdName, metav1.GetOptions{}). Update all callers of that function to pass their context (e.g., the request or main ctx) and adjust any tests/mocks to supply a context so callers can cancel or timeout CRD fetches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/export/export.go`:
- Around line 158-162: The log call that prints CRD export count (log.Infof)
uses a format string with a trailing "\n", causing double newlines because
Logrus already appends one; update the log invocation that references crdCount
and crdResources to remove the trailing "\n" from the format string (i.e., call
log.Infof with "Exported %d CRDs for referenced custom resources to the _cluster
resources directory" and keep the crdCount argument).
---
Nitpick comments:
In `@cmd/export/crd.go`:
- Around line 84-123: The loop over the map variable seen yields
non-deterministic CRD order; change the iteration to collect keys from seen into
a slice, sort that slice (e.g., using sort.Strings), and then iterate over the
sorted slice (using crdName from the sorted keys) when calling crdClient.Get and
appending to out/outErrs; update any references inside the loop that assume the
original map iteration but keep the existing error handling and log messages in
the block that uses crdName, crdClient.Get, outErrs, and out.
- Line 85: The code uses context.Background() when calling crdClient.Get, which
prevents cancellation; change the enclosing function to accept a context.Context
parameter (replace context.Background() with the passed ctx) and propagate that
ctx through to crdClient.Get(ctx, crdName, metav1.GetOptions{}). Update all
callers of that function to pass their context (e.g., the request or main ctx)
and adjust any tests/mocks to supply a context so callers can cancel or timeout
CRD fetches.
🪄 Autofix (Beta)
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
Run ID: 7521d672-60df-4336-8b98-1606da0f109f
📒 Files selected for processing (3)
cmd/export/crd.gocmd/export/crd_test.gocmd/export/export.go
Contributor
Author
Updated SummaryWhat’s changed
Effective precedence (implemented)
Files updated
Behavior impact
Validation
|
Contributor
Author
|
This PR is dependent on migtools/crane-lib#136 so Unit tests will keep failing until that one is merged. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/export/crd.go (2)
17-24: Normalize user-provided group names to lowercase.
normalizeGroupSettrims whitespace, but case mismatches in CLI values can silently bypass include/skip matching.Diff suggestion
func normalizeGroupSet(groups []string) map[string]struct{} { out := map[string]struct{}{} for _, g := range groups { - g = strings.TrimSpace(g) + g = strings.ToLower(strings.TrimSpace(g)) if g == "" { continue } out[g] = struct{}{} } return out }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/export/crd.go` around lines 17 - 24, normalizeGroupSet currently trims whitespace but doesn't normalize case, so CLI group names can mismatch; update normalizeGroupSet to convert each trimmed group string to a canonical lowercase form (use strings.ToLower) before checking for empty and adding to the out map. Ensure you call strings.TrimSpace first, then strings.ToLower on the result, and continue to deduplicate via out[g] = struct{}{}; reference the normalizeGroupSet function to apply this change.
84-84: Consider deterministic CRD processing order.Iterating
seenmap directly makes success/error ordering nondeterministic; sorting keys would stabilize logs/output behavior.Diff suggestion
import ( "context" "fmt" + "sort" "strings" @@ - for crdName := range seen { + keys := make([]string, 0, len(seen)) + for crdName := range seen { + keys = append(keys, crdName) + } + sort.Strings(keys) + + for _, crdName := range keys { obj, err := crdClient.Get(context.Background(), crdName, metav1.GetOptions{})Also applies to: 124-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/export/crd.go` at line 84, The loop "for crdName := range seen" in cmd/export/crd.go yields nondeterministic ordering; collect the map keys from seen into a slice, sort them (e.g., with sort.Strings), and then iterate over the sorted slice instead of ranging the map to produce deterministic CRD processing and logs; apply the same change for the similar loop at the second occurrence (around the other "for crdName := range seen").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/export/crd.go`:
- Around line 17-24: normalizeGroupSet currently trims whitespace but doesn't
normalize case, so CLI group names can mismatch; update normalizeGroupSet to
convert each trimmed group string to a canonical lowercase form (use
strings.ToLower) before checking for empty and adding to the out map. Ensure you
call strings.TrimSpace first, then strings.ToLower on the result, and continue
to deduplicate via out[g] = struct{}{}; reference the normalizeGroupSet function
to apply this change.
- Line 84: The loop "for crdName := range seen" in cmd/export/crd.go yields
nondeterministic ordering; collect the map keys from seen into a slice, sort
them (e.g., with sort.Strings), and then iterate over the sorted slice instead
of ranging the map to produce deterministic CRD processing and logs; apply the
same change for the similar loop at the second occurrence (around the other "for
crdName := range seen").
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8cecf9b-a00c-4f40-9629-ba9e3f74374e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
cmd/export/crd.gocmd/export/crd_test.gocmd/export/export.gogo.mod
✅ Files skipped from review due to trivial changes (2)
- go.mod
- cmd/export/export.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/export/crd_test.go
This was referenced Jun 11, 2026
4 tasks
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.
Summary
crane exportnow also saves CRD YAML for custom API types that show up in the export (so you can apply them on a new cluster without pulling every CRD from the source).What it does
-cis used), for each custom API group that has objects in the export, Crane GETs the matching CustomResourceDefinition (plural.group) and writes it underresources/<ns>/_cluster/.failures/<ns>/like other export errors.Tests
cmd/export/crd_test.go— built-in group checks;collectRelatedCRDswith a fake dynamic client (success, built-in skip, dedupe, subresource skip, multiple CRDs, GET failure → failures path).crane-complex-demo: Widget (stable.example.com/v1, CRDwidgets.stable.example.com), plus Deployment, Service, ConfigMap, ServiceAccount, Secret, ClusterRole / ClusterRoleBinding bound to that SA; on OpenShift, optional SCC from the same demo. Rancrane exportwith--cluster-scoped-rbacand confirmed CRD + RBAC (and SCC when present) and workload + CR manifests too.Notes
*.openshift.ioare not fetched as CRDs by this path (by design).Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Tests