Skip to content

Commit

Permalink
Add crds.yaml-based defaulting functions to pkg/apis/templates/... (#129
Browse files Browse the repository at this point in the history
)

Each ConstrainTemplate version (v1alpha1, v1beta1, v1) has a default
value for the LegacySchema field. true for v1alpha1, v1beta1, and
false for v1. These defaults are defined in the CRD yaml.

For the ConstraintTemplate golang types to have the same functionality
to have the same defaulting functionality. We are required to write
custom defaulting functions for each defaulted field.

While we could do this, we could easily see how this practice could
become unsustainable. A forgotten defaulting function would yield
unexpected bugs.

To remedy this, this PR adds defaulting functions that default based on
the content of deploy/crds.yaml, i.e. the ConstraintTemplate CRD
generated by controller-gen.

This provides us with a single defaulting source of truth: the
constrainttemplate_types.go files, which contain annotations that
define a field's default values.

Fixes #128

Signed-off-by: juliankatz <juliankatz@google.com>
  • Loading branch information
julianKatz authored Aug 16, 2021
1 parent d4a673c commit 2924b2c
Show file tree
Hide file tree
Showing 45 changed files with 1,214 additions and 270 deletions.
8 changes: 5 additions & 3 deletions constraint/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ RUN curl -L -O "https://github.com/kubernetes-sigs/kubebuilder/releases/download
ENV PATH=$PATH:/usr/local/kubebuilder/bin:/usr/bin

# Install kustomize
ENV version=3.0.2
ENV version=3.8.9
ENV arch=amd64
RUN curl -L -O "https://github.com/kubernetes-sigs/kustomize/releases/download/v${version}/kustomize_${version}_linux_${arch}" &&\
mv kustomize_${version}_linux_${arch} /usr/bin/kustomize &&\
ENV tar_name=kustomize_v${version}_linux_${arch}.tar.gz
RUN curl -LO "https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize%2Fv${version}/${tar_name}" &&\
tar -xf ${tar_name} &&\
mv ./kustomize /usr/bin/kustomize &&\
chmod u+x /usr/bin/kustomize

# Copy in the go src
Expand Down
36 changes: 33 additions & 3 deletions constraint/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ docker-internal-test:

# Hook to run docker tests
test:
docker build . -t constraint-test && docker run -t constraint-test
docker build . -t constraint-test && docker run -it constraint-test

# Install CRDs into a cluster
install: manifests
Expand All @@ -32,7 +32,8 @@ install: manifests
# Install the generation dependencies on the local machine
gen-dependencies:
go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.5.0
go install k8s.io/code-generator/cmd/conversion-gen@v0.20.2
go install k8s.io/code-generator/cmd/conversion-gen@v0.21.3
go install k8s.io/code-generator/cmd/defaulter-gen@v0.21.3

# Generate manifests e.g. CRD, RBAC etc.
manifests:
Expand All @@ -47,7 +48,7 @@ lint:

# Generate code
# Not working? Try running `make gen-dependencies`
generate:
generate: generate-defaults
controller-gen \
object:headerFile=./hack/boilerplate.go.txt \
paths="./pkg/..."
Expand All @@ -61,3 +62,32 @@ generate:
--go-header-file=./hack/boilerplate.go.txt \
--output-file-base=zz_generated.conversion \
--extra-dirs=k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1

CRD_SOURCE_FILE := deploy/crds.yaml
FILE_STUB := "package templates\
\n\
\n// This file is generated from $(CRD_SOURCE_FILE) via \"make constraint-template-string-constant\"\
\n// DO NOT MODIFY THIS FILE DIRECTLY!\
\n\
\nconst constraintTemplateCRDYaml = \`"
YAML_CONSTANT_GOLANG_FILE := ./pkg/apis/templates/yaml_constant.go
constraint-template-string-constant: manifests
rm -rf $(YAML_CONSTANT_GOLANG_FILE)
bash -c 'echo -en ${FILE_STUB} >> ${YAML_CONSTANT_GOLANG_FILE}'
bash -c 'cat deploy/crds.yaml >> ${YAML_CONSTANT_GOLANG_FILE}'
bash -c 'echo "\`" >> ${YAML_CONSTANT_GOLANG_FILE}'
# Remove trailing spaces. Double $ is to prevent variable expansion in make
sed -i "s/ $$//g" ${YAML_CONSTANT_GOLANG_FILE}
generate-defaults: constraint-template-string-constant
defaulter-gen \
--input-dirs "./pkg/apis/templates/..." \
--go-header-file=./hack/boilerplate.go.txt \
--output-file-base=zz_generated.defaults
.PHONY: vendor
vendor:
go mod vendor
go mod tidy
1 change: 1 addition & 0 deletions constraint/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ require (
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.19 // indirect
sigs.k8s.io/controller-runtime v0.8.3
sigs.k8s.io/structured-merge-diff/v4 v4.1.1 // indirect
sigs.k8s.io/yaml v1.2.0
)
39 changes: 39 additions & 0 deletions constraint/pkg/apis/templates/crd_schema.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package templates

import (
"fmt"

"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"sigs.k8s.io/yaml"
)

var ConstraintTemplateSchemas map[string]*schema.Structural

func initializeCTSchemaMap() {
// Setup the CT Schema map for use in generalized defaulting functions
ConstraintTemplateSchemas = make(map[string]*schema.Structural)

// Ingest the constraint template CRD for use in defaulting functions
constraintTemplateCRD := &apiextensionsv1.CustomResourceDefinition{}
if err := yaml.Unmarshal([]byte(constraintTemplateCRDYaml), constraintTemplateCRD); err != nil {
panic(fmt.Errorf("%w: failed to unmarshal yaml into constraintTemplateCRD", err))
}

// Fill version map with Structural types derived from ConstraintTemplate versions
for _, crdVersion := range constraintTemplateCRD.Spec.Versions {
versionlessSchema := &apiextensions.JSONSchemaProps{}
err := Scheme.Convert(crdVersion.Schema.OpenAPIV3Schema, versionlessSchema, nil)
if err != nil {
panic(fmt.Errorf("%w: failed to convert JSONSchemaProps for ConstraintTemplate version %v", err, crdVersion.Name))
}

structural, err := schema.NewStructural(versionlessSchema)
if err != nil {
panic(fmt.Errorf("%w: failed to create Structural for ConstraintTemplate version %v", err, crdVersion.Name))
}

ConstraintTemplateSchemas[crdVersion.Name] = structural
}
}
12 changes: 12 additions & 0 deletions constraint/pkg/apis/templates/init.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package templates

// While multiple `init()` functions might seem like the way to handle this, the dependency between
// the two functions makes this a poor solution. Golang orders the files in a package
// lexicographically and then runs their init() functions in that order. As crd_scheme.go comes
// before scheme.go, initializeCTSchemaMap() was run before initializeSchema(). This caused a
// null-pointer exception, as the Scheme used throughout the package hadn't yet been initialized.
// Calling them in order here fixes that problem.
func init() {
initializeScheme()
initializeCTSchemaMap()
}
19 changes: 19 additions & 0 deletions constraint/pkg/apis/templates/scheme.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package templates

import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
)

var Scheme *runtime.Scheme

func initializeScheme() {
Scheme = runtime.NewScheme()
if err := apiextensionsv1.AddToScheme(Scheme); err != nil {
panic(err)
}
if err := apiextensions.AddToScheme(Scheme); err != nil {
panic(err)
}
}
75 changes: 75 additions & 0 deletions constraint/pkg/apis/templates/test_fakes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package templates

import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)

func VersionedIncompleteSchema() *apiextensionsv1.JSONSchemaProps {
return &apiextensionsv1.JSONSchemaProps{
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"message": {
Type: "string",
},
"labels": {
Type: "array",
Items: &apiextensionsv1.JSONSchemaPropsOrArray{
Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{
"key": {Type: "string"},
"allowedRegex": {Type: "string"},
},
},
},
},
},
}
}

func VersionlessSchemaWithXPreserve() *apiextensions.JSONSchemaProps {
trueBool := true
return &apiextensions.JSONSchemaProps{
XPreserveUnknownFields: &trueBool,
Properties: map[string]apiextensions.JSONSchemaProps{
"message": {
Type: "string",
},
"labels": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
XPreserveUnknownFields: &trueBool,
Properties: map[string]apiextensions.JSONSchemaProps{
"key": {Type: "string"},
"allowedRegex": {Type: "string"},
},
},
},
},
},
}
}

func VersionlessSchema() *apiextensions.JSONSchemaProps {
return &apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"message": {
Type: "string",
},
"labels": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"key": {Type: "string"},
"allowedRegex": {Type: "string"},
},
},
},
},
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Validation struct {
// +kubebuilder:pruning:PreserveUnknownFields
OpenAPIV3Schema *apiextensionsv1.JSONSchemaProps `json:"openAPIV3Schema,omitempty"`
// +kubebuilder:default=false
LegacySchema bool `json:"legacySchema,omitempty"`
LegacySchema *bool `json:"legacySchema,omitempty"` // *bool allows for "unset" state which we need to apply appropriate defaults
}

type Target struct {
Expand Down
Loading

0 comments on commit 2924b2c

Please sign in to comment.