Move CRD defaulting helpers outside of pkg/apis (#138)

In the building of `gator` github.com/willbeason found that the usage of
a shared Scheme amongst multiple sub-packages of pkg/apis/templates was
not thread safe.  An internal map within the Scheme was experiencing a
simultaneous read and write.

Instantiating the scheme in a super-package and using it in the
sub-packages required the creation of a circular dependency, as the
super-package needed the contents of the sub-package to create a
functioning scheme that could convert between sub-package types.  This
was previously solved by adding sub-package contents to the scheme at
scheme-usage-time, but this yielded the race condition on the map.

This PR moves most of the core logic into a separate package: pkg/schema

To prevent a circular dependency, a *runtime.Scheme is passed from
consumer packages to this new package's function call.

These schemes are instantiated once in the consumer packages and the
output of pkg/schema is stored at consumer package init() time to
prevent unnecessary re-calculation.

Fixes #137

Signed-off-by: juliankatz <juliankatz@google.com>

constraint/Makefile

@@ -64,14 +64,14 @@ generate: generate-defaults
 		--extra-dirs=k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
 
 CRD_SOURCE_FILE := deploy/crds.yaml
-FILE_STUB := "package templates\
+FILE_STUB := "package schema\
 \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
+YAML_CONSTANT_GOLANG_FILE := ./pkg/schema/yaml_constant.go
 
 constraint-template-string-constant: manifests
 	rm -rf $(YAML_CONSTANT_GOLANG_FILE)

constraint/pkg/apis/templates/v1/constrainttemplate_types_test.go

@@ -21,8 +21,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/onsi/gomega"
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"golang.org/x/net/context"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -155,31 +155,31 @@ func TestValidationVersionConversionAndTransformation(t *testing.T) {
 			name: "Two deep properties, LegacySchema=true",
 			v: &Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchemaWithXPreserve(),
+				OpenAPIV3Schema: schema.VersionlessSchemaWithXPreserve(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=false",
 			v: &Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=nil",
 			v: &Validation{
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{

constraint/pkg/apis/templates/v1/conversion.go

@@ -16,8 +16,8 @@ limitations under the License.
 package v1
 
 import (
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	coreTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/conversion"
@@ -35,7 +35,7 @@ func Convert_v1_Validation_To_templates_Validation(in *Validation, out *coreTemp
 		inSchemaCopy := inSchema.DeepCopy()
 
 		if in.LegacySchema != nil && *in.LegacySchema {
-			if err := apisTemplates.AddPreserveUnknownFields(inSchemaCopy); err != nil {
+			if err := schema.AddPreserveUnknownFields(inSchemaCopy); err != nil {
 				return err
 			}
 		}

constraint/pkg/apis/templates/v1/defaults.go

@@ -1,13 +1,38 @@
 package v1
 
 import (
-	"github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
+	ctschema "github.com/open-policy-agent/frameworks/constraint/pkg/schema"
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
 const version = "v1"
 
+var (
+	structuralSchema *schema.Structural
+	Scheme           *runtime.Scheme
+)
+
+func init() {
+	Scheme = runtime.NewScheme()
+	var err error
+	if err = apiextensionsv1.AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if err = apiextensions.AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if err = AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if structuralSchema, err = ctschema.CRDSchema(Scheme, version); err != nil {
+		panic(err)
+	}
+}
+
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
 	return RegisterDefaults(scheme)
 }
@@ -19,7 +44,7 @@ func SetDefaults_ConstraintTemplate(obj *ConstraintTemplate) { // nolint:revive
 		panic("Failed to convert v1 ConstraintTemplate to Unstructured")
 	}
 
-	defaulting.Default(un, templates.ConstraintTemplateSchemas[version])
+	defaulting.Default(un, structuralSchema)
 
 	err = runtime.DefaultUnstructuredConverter.FromUnstructured(un, obj)
 	if err != nil {

constraint/pkg/apis/templates/v1/helpers.go

@@ -1,22 +1,17 @@
 package v1
 
 import (
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
 )
 
 // ToVersionless runs defaulting functions and then converts the ConstraintTemplate to the
 // versionless api representation.
 func (versioned *ConstraintTemplate) ToVersionless() (*templates.ConstraintTemplate, error) {
-	if err := AddToScheme(apisTemplates.Scheme); err != nil {
-		return nil, err
-	}
-
 	versionedCopy := versioned.DeepCopy()
-	apisTemplates.Scheme.Default(versionedCopy)
+	Scheme.Default(versionedCopy)
 
 	versionless := &templates.ConstraintTemplate{}
-	if err := apisTemplates.Scheme.Convert(versionedCopy, versionless, nil); err != nil {
+	if err := Scheme.Convert(versionedCopy, versionless, nil); err != nil {
 		return nil, err
 	}
 

constraint/pkg/apis/templates/v1alpha1/constrainttemplate_types_test.go

@@ -21,8 +21,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/onsi/gomega"
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"golang.org/x/net/context"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -150,31 +150,31 @@ func TestValidationVersionConversionAndTransformation(t *testing.T) {
 			name: "Two deep properties, LegacySchema=true",
 			v: &Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchemaWithXPreserve(),
+				OpenAPIV3Schema: schema.VersionlessSchemaWithXPreserve(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=false",
 			v: &Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=nil",
 			v: &Validation{
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{

constraint/pkg/apis/templates/v1alpha1/conversion.go

@@ -16,8 +16,8 @@ limitations under the License.
 package v1alpha1
 
 import (
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	coreTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/conversion"
@@ -35,7 +35,7 @@ func Convert_v1alpha1_Validation_To_templates_Validation(in *Validation, out *co
 		inSchemaCopy := inSchema.DeepCopy()
 
 		if in.LegacySchema != nil && *in.LegacySchema {
-			if err := apisTemplates.AddPreserveUnknownFields(inSchemaCopy); err != nil {
+			if err := schema.AddPreserveUnknownFields(inSchemaCopy); err != nil {
 				return err
 			}
 		}

constraint/pkg/apis/templates/v1alpha1/defaults.go

@@ -1,13 +1,38 @@
 package v1alpha1
 
 import (
-	"github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
+	ctschema "github.com/open-policy-agent/frameworks/constraint/pkg/schema"
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
 const version = "v1alpha1"
 
+var (
+	structuralSchema *schema.Structural
+	Scheme           *runtime.Scheme
+)
+
+func init() {
+	Scheme = runtime.NewScheme()
+	var err error
+	if err = apiextensionsv1.AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if err = apiextensions.AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if err = AddToScheme(Scheme); err != nil {
+		panic(err)
+	}
+	if structuralSchema, err = ctschema.CRDSchema(Scheme, version); err != nil {
+		panic(err)
+	}
+}
+
 func addDefaultingFuncs(scheme *runtime.Scheme) error {
 	return RegisterDefaults(scheme)
 }
@@ -19,7 +44,7 @@ func SetDefaults_ConstraintTemplate(obj *ConstraintTemplate) { // nolint:revive
 		panic("Failed to convert v1 ConstraintTemplate to Unstructured")
 	}
 
-	defaulting.Default(un, templates.ConstraintTemplateSchemas[version])
+	defaulting.Default(un, structuralSchema)
 
 	err = runtime.DefaultUnstructuredConverter.FromUnstructured(un, obj)
 	if err != nil {

constraint/pkg/apis/templates/v1alpha1/helpers.go

@@ -1,22 +1,17 @@
 package v1alpha1
 
 import (
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
 )
 
 // ToVersionless runs defaulting functions and then converts the ConstraintTemplate to the
 // versionless api representation.
 func (versioned *ConstraintTemplate) ToVersionless() (*templates.ConstraintTemplate, error) {
-	if err := AddToScheme(apisTemplates.Scheme); err != nil {
-		return nil, err
-	}
-
 	versionedCopy := versioned.DeepCopy()
-	apisTemplates.Scheme.Default(versionedCopy)
+	Scheme.Default(versionedCopy)
 
 	versionless := &templates.ConstraintTemplate{}
-	if err := apisTemplates.Scheme.Convert(versionedCopy, versionless, nil); err != nil {
+	if err := Scheme.Convert(versionedCopy, versionless, nil); err != nil {
 		return nil, err
 	}
 

constraint/pkg/apis/templates/v1beta1/constrainttemplate_types_test.go

@@ -21,8 +21,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/onsi/gomega"
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"golang.org/x/net/context"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -149,31 +149,31 @@ func TestValidationVersionConversionAndTransformation(t *testing.T) {
 			name: "Two deep properties, LegacySchema=true",
 			v: &Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &trueBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchemaWithXPreserve(),
+				OpenAPIV3Schema: schema.VersionlessSchemaWithXPreserve(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=false",
 			v: &Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
 				LegacySchema:    &falseBool,
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{
 			name: "Two deep properties, LegacySchema=nil",
 			v: &Validation{
-				OpenAPIV3Schema: apisTemplates.VersionedIncompleteSchema(),
+				OpenAPIV3Schema: schema.VersionedIncompleteSchema(),
 			},
 			exp: &templates.Validation{
-				OpenAPIV3Schema: apisTemplates.VersionlessSchema(),
+				OpenAPIV3Schema: schema.VersionlessSchema(),
 			},
 		},
 		{

constraint/pkg/apis/templates/v1beta1/conversion.go

@@ -16,8 +16,8 @@ limitations under the License.
 package v1beta1
 
 import (
-	apisTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates"
 	coreTemplates "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
+	"github.com/open-policy-agent/frameworks/constraint/pkg/schema"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/conversion"
@@ -35,7 +35,7 @@ func Convert_v1beta1_Validation_To_templates_Validation(in *Validation, out *cor
 		inSchemaCopy := inSchema.DeepCopy()
 
 		if in.LegacySchema != nil && *in.LegacySchema {
-			if err := apisTemplates.AddPreserveUnknownFields(inSchemaCopy); err != nil {
+			if err := schema.AddPreserveUnknownFields(inSchemaCopy); err != nil {
 				return err
 			}
 		}