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

⚠ Generate Embedded ObjectMeta in the CRDs, plus addressed comments #557

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
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ require (
k8s.io/apimachinery v0.20.2
sigs.k8s.io/yaml v1.2.0
)

20 changes: 20 additions & 0 deletions pkg/crd/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ type Generator struct {
// You'll need to use "v1" to get support for features like defaulting,
// along with an API server that supports it (Kubernetes 1.16+).
CRDVersions []string `marker:"crdVersions,optional"`

// GenerateEmbeddedObjectMeta specifies if any embedded ObjectMeta in the CRD should be generated
GenerateEmbeddedObjectMeta *bool `marker:",optional"`
}

func (Generator) CheckFilter() loader.NodeFilter {
Expand All @@ -98,6 +101,8 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
Checker: ctx.Checker,
// Perform defaulting here to avoid ambiguity later
AllowDangerousTypes: g.AllowDangerousTypes != nil && *g.AllowDangerousTypes == true,
// Indicates the parser on whether to register the ObjectMeta type or not
GenerateEmbeddedObjectMeta: g.GenerateEmbeddedObjectMeta != nil && *g.GenerateEmbeddedObjectMeta == true,
}

AddKnownTypes(parser)
Expand Down Expand Up @@ -129,6 +134,9 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
crdRaw := parser.CustomResourceDefinitions[groupKind]
addAttribution(&crdRaw)

// Prevent the top level metadata for the CRD to be generate regardless of the intention in the arguments
FixTopLevelMetadata(crdRaw)

versionedCRDs := make([]interface{}, len(crdVersions))
for i, ver := range crdVersions {
conv, err := AsVersion(crdRaw, schema.GroupVersion{Group: apiext.SchemeGroupVersion.Group, Version: ver})
Expand Down Expand Up @@ -269,6 +277,18 @@ func removeDefaultsFromSchemaProps(v *apiextlegacy.JSONSchemaProps) {
}
}

// FixTopLevelMetadata resets the schema for the top-level metadata field which is needed for CRD validation
func FixTopLevelMetadata(crd apiext.CustomResourceDefinition) {
for _, v := range crd.Spec.Versions {
if v.Schema != nil && v.Schema.OpenAPIV3Schema != nil && v.Schema.OpenAPIV3Schema.Properties != nil {
schemaProperties := v.Schema.OpenAPIV3Schema.Properties
if _, ok := schemaProperties["metadata"]; ok {
schemaProperties["metadata"] = apiext.JSONSchemaProps{Type: "object"}
}
}
}
}

// toTrivialVersions strips out all schemata except for the storage schema,
// and moves that up into the root object. This makes the CRD compatible
// with pre 1.13 clusters.
Expand Down
55 changes: 53 additions & 2 deletions pkg/crd/known_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ var KnownPackages = map[string]PackageOverride{
},

"k8s.io/apimachinery/pkg/apis/meta/v1": func(p *Parser, pkg *loader.Package) {
// ObjectMeta is managed by the Kubernetes API server, so no need to
// generate validation for it.
p.Schemata[TypeIdent{Name: "ObjectMeta", Package: pkg}] = apiext.JSONSchemaProps{
Type: "object",
}
Expand Down Expand Up @@ -113,6 +111,53 @@ var KnownPackages = map[string]PackageOverride{
},
}

// ObjectMetaPackages overrides the ObjectMeta in all types
var ObjectMetaPackages = map[string]PackageOverride{
"k8s.io/apimachinery/pkg/apis/meta/v1": func(p *Parser, pkg *loader.Package) {
// execute the KnowPackages for `k8s.io/apimachinery/pkg/apis/meta/v1` if any
if f, ok := KnownPackages["k8s.io/apimachinery/pkg/apis/meta/v1"]; ok {
f(p, pkg)
}
// This is an allow-listed set of properties of ObjectMeta, other runtime properties are not part of this list
// See more discussion: https://github.com/kubernetes-sigs/controller-tools/pull/395#issuecomment-691919433
p.Schemata[TypeIdent{Name: "ObjectMeta", Package: pkg}] = apiext.JSONSchemaProps{
Type: "object",
Properties: map[string]apiext.JSONSchemaProps{
"name": apiext.JSONSchemaProps{
Type: "string",
},
"namespace": apiext.JSONSchemaProps{
Type: "string",
},
"annotations": apiext.JSONSchemaProps{
Type: "object",
AdditionalProperties: &apiext.JSONSchemaPropsOrBool{
Schema: &apiext.JSONSchemaProps{
Type: "string",
},
},
},
"labels": apiext.JSONSchemaProps{
Type: "object",
AdditionalProperties: &apiext.JSONSchemaPropsOrBool{
Schema: &apiext.JSONSchemaProps{
Type: "string",
},
},
},
"finalizers": apiext.JSONSchemaProps{
Copy link
Contributor

Choose a reason for hiding this comment

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

this is odd. Why finalizers?

Choose a reason for hiding this comment

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

I had the same thought, I'd have expected either all the ObjectMeta fields or the non-system fields here https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/object-meta/

Copy link
Member

Choose a reason for hiding this comment

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

@jessefeinman Can you open an issue to discuss this?

Choose a reason for hiding this comment

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

Type: "array",
Items: &apiext.JSONSchemaPropsOrArray{
Schema: &apiext.JSONSchemaProps{
Type: "string",
},
},
},
},
}
},
}

func boolPtr(b bool) *bool {
return &b
}
Expand All @@ -125,4 +170,10 @@ func AddKnownTypes(parser *Parser) {
for pkgName, override := range KnownPackages {
parser.PackageOverrides[pkgName] = override
}
// if we want to generate the embedded ObjectMeta in the CRD we need to add the ObjectMetaPackages
if parser.GenerateEmbeddedObjectMeta {
for pkgName, override := range ObjectMetaPackages {
parser.PackageOverrides[pkgName] = override
}
}
}
3 changes: 3 additions & 0 deletions pkg/crd/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ type Parser struct {
// because the implementation is too difficult/clunky to promote them to category 3.
// TODO: Should we have a more formal mechanism for putting "type patterns" in each of the above categories?
AllowDangerousTypes bool

// GenerateEmbeddedObjectMeta specifies if any embedded ObjectMeta should be generated
GenerateEmbeddedObjectMeta bool
}

func (p *Parser) init() {
Expand Down
6 changes: 6 additions & 0 deletions pkg/crd/parser_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
groupKind := schema.GroupKind{Kind: "CronJob", Group: "testdata.kubebuilder.io"}
parser.NeedCRDFor(groupKind, nil)

By("fixing top level ObjectMeta on the CRD")
crd.FixTopLevelMetadata(parser.CustomResourceDefinitions[groupKind])

By("checking that no errors occurred along the way (expect for type errors)")
Expect(packageErrors(cronJobPkg, packages.TypeError)).NotTo(HaveOccurred())

Expand Down Expand Up @@ -133,6 +136,9 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
groupKind := schema.GroupKind{Kind: "TestQuota", Group: "plural.example.com"}
parser.NeedCRDFor(groupKind, nil)

By("fixing top level ObjectMeta on the CRD")
crd.FixTopLevelMetadata(parser.CustomResourceDefinitions[groupKind])

By("loading the desired YAML")
expectedFile, err := ioutil.ReadFile("plural.example.com_testquotas.yaml")
Expect(err).NotTo(HaveOccurred())
Expand Down
4 changes: 4 additions & 0 deletions pkg/crd/zz_generated.markerhelp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.