fix: CRD generation fails for schema property named "group" (unknown type Group) - #48
Closed
braghettos wants to merge 1 commit into
Closed
Conversation
A schema property named after an identifier that crdgen emits at package scope (most notably "group", which becomes the "Group" constant in groupversion_info.go) produced a struct type of the same name in the same package. The resulting redeclaration made the field reference resolve to the constant instead of the type, so CRD generation failed with "unknown type Group". Rename such generated struct types (e.g. Group -> GroupEnvelope) while keeping the field name and JSON tag unchanged, and add a regression test covering group/version plus previously-working names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
Author
|
Moving this to a PR within the braghettos fork instead of upstream. |
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
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.
The bug
When an OpenAPI/JSON schema has a property literally named
group, crdgen fails to generate the CRD. controller-gen aborts with:This happens whether the sub-schema is
$ref'd or inlined — the literal property namegroupis the trigger. Other envelope names (project,user,role,domain, ...) work fine. The concrete case is the OpenStack Keystone Group resource, whose request body is wrapped in a top-levelgroupenvelope (spec.group.*).Root cause
The generated Go type for the
groupproperty and the generatedapispackage share the same package scope:groupversion_info.goemits a package-level constantconst Group = "<api group>".types.goemitstype Group struct { ... }for thegroupenvelope property, in the same package.Two declarations of
Groupin one package is a redeclaration; the fieldGroup *Groupthen resolvesGroupto the constant rather than a type, so controller-gen reportsunknown type Group. The same latent collision exists for any property named after an identifier crdgen emits at package scope (Group,Version,SchemeGroupVersion,SchemeBuilder,AddToScheme,AddToSchemes).The fix
In the transpiler, sanitize the generated struct type name (not the field name or JSON tag) so it never equals one of those reserved package-level identifiers. Colliding names get an
Envelopesuffix (Group->GroupEnvelope), chosen to also avoid clashing with the<Kind>Spec/<Kind>Status/<Kind>Listtypes crdgen generates. Because both the struct definition and the field's type reference derive from the same name insideprocessObject, they stay consistent.The field name and JSON tag are untouched, so the CRD still exposes
spec.group.*exactly as before — only the internal Go type is renamed.Verification
Before: CRD generation fails with
unknown type Group.After: the Keystone
groupenvelope generates a valid CRD withspec.group.{name,domain_id,description}and the correctrequiredmarkers.Added a deterministic regression test (
TestReservedTypeNameCollision) coveringgroupandversion(which must be renamed) alongsideproject/domain/user/role(which must not), asserting the field type references a defined struct and the JSON name is preserved.🤖 Generated with Claude Code