Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Don't generate description on metadata of CRD structural schema #511

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions pkg/crd/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,14 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
}

for i, crd := range versionedCRDs {
// defaults are not allowed to be specified in v1beta1 CRDs, so strip them
// before writing to a file
// defaults are not allowed to be specified in v1beta1 CRDs and
// decriptions are not allowed on the metadata regardless of version
// strip them before writing to a file
if crdVersions[i] == "v1beta1" {
removeDefaultsFromSchemas(crd.(*apiextlegacy.CustomResourceDefinition))
removeDescriptionFromMetadataLegacy(crd.(*apiextlegacy.CustomResourceDefinition))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not loving the duplicated functions for v1 and v1beta1, but I couldn't find a cleaner way to make it work. I thought of passing the generic runtime.Object into a single removeDescriptionFromMetadata() function, but it didn't seem any better since we already have to do the crdVersions check for removing defaults. Let me know if there's a better way.

} else {
removeDescriptionFromMetadata(crd.(*apiext.CustomResourceDefinition))
}
var fileName string
if i == 0 {
Expand All @@ -181,6 +185,47 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
return nil
}

func removeDescriptionFromMetadata(crd *apiext.CustomResourceDefinition) {
for _, versionSpec := range crd.Spec.Versions {
if versionSpec.Schema != nil {
removeDescriptionFromMetadataProps(versionSpec.Schema.OpenAPIV3Schema)
}
}
}

func removeDescriptionFromMetadataProps(v *apiext.JSONSchemaProps) {
if m, ok := v.Properties["metadata"]; ok {
meta := &m
if meta.Description != "" {
meta.Description = ""
v.Properties["metadata"] = m

}
}
}

func removeDescriptionFromMetadataLegacy(crd *apiextlegacy.CustomResourceDefinition) {
if crd.Spec.Validation != nil {
removeDescriptionFromMetadataPropsLegacy(crd.Spec.Validation.OpenAPIV3Schema)
}
for _, versionSpec := range crd.Spec.Versions {
if versionSpec.Schema != nil {
removeDescriptionFromMetadataPropsLegacy(versionSpec.Schema.OpenAPIV3Schema)
}
}
}

func removeDescriptionFromMetadataPropsLegacy(v *apiextlegacy.JSONSchemaProps) {
if m, ok := v.Properties["metadata"]; ok {
meta := &m
if meta.Description != "" {
meta.Description = ""
v.Properties["metadata"] = m

}
}
}

// removeDefaultsFromSchemas will remove all instances of default values being
// specified across all defined API versions
func removeDefaultsFromSchemas(crd *apiextlegacy.CustomResourceDefinition) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/crd/gen_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var _ = Describe("CRD Generation proper defaulting", func() {
}
})

It("should strip v1beta1 CRDs of default fields", func() {
It("should strip v1beta1 CRDs of default fields and metadata description", func() {
By("calling Generate")
gen := &crd.Generator{
CRDVersions: []string{"v1beta1"},
Expand All @@ -84,7 +84,7 @@ var _ = Describe("CRD Generation proper defaulting", func() {

})

It("should not strip v1 CRDs of default fields", func() {
It("should not strip v1 CRDs of default fields and metadata description", func() {
By("calling Generate")
gen := &crd.Generator{
CRDVersions: []string{"v1"},
Expand Down
2 changes: 2 additions & 0 deletions pkg/crd/testdata/gen/bar.example.com_foos.v1beta1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ spec:
metadata:
type: object
spec:
description: Spec comments SHOULD appear in the CRD spec
properties:
defaultedString:
description: This tests that defaulted fields are stripped for v1beta1, but not for v1
Expand All @@ -35,6 +36,7 @@ spec:
- defaultedString
type: object
status:
description: Status comments SHOULD appear in the CRD spec
type: object
type: object
version: foo
Expand Down
2 changes: 2 additions & 0 deletions pkg/crd/testdata/gen/bar.example.com_foos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ spec:
metadata:
type: object
spec:
description: Spec comments SHOULD appear in the CRD spec
properties:
defaultedString:
default: fooDefaultString
Expand All @@ -38,6 +39,7 @@ spec:
- defaultedString
type: object
status:
description: Status comments SHOULD appear in the CRD spec
type: object
type: object
served: true
Expand Down
8 changes: 6 additions & 2 deletions pkg/crd/testdata/gen/foo_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,13 @@ type FooSpec struct {
type FooStatus struct{}

type Foo struct {
metav1.TypeMeta `json:",inline"`
// TypeMeta comments should NOT appear in the CRD spec
metav1.TypeMeta `json:",inline"`
// ObjectMeta comments should NOT appear in the CRD spec
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec FooSpec `json:"spec,omitempty"`
// Spec comments SHOULD appear in the CRD spec
Spec FooSpec `json:"spec,omitempty"`
// Status comments SHOULD appear in the CRD spec
Status FooStatus `json:"status,omitempty"`
}