Merge pull request #776 from shravanshetty1/765_globalstate

#765 - Removing global state from openAPI

cmd/kyverno/main.go

@@ -250,8 +250,14 @@ func main() {
 		os.Exit(1)
 	}
 
+	openAPIController, err := openapi.NewOpenAPIController()
+	if err != nil {
+		setupLog.Error(err, "Failed to create openAPIController")
+		os.Exit(1)
+	}
+
 	// Sync openAPI definitions of resources
-	openApiSync := openapi.NewCRDSync(client)
+	openApiSync := openapi.NewCRDSync(client, openAPIController)
 
 	// WEBHOOOK
 	// - https server to provide endpoints called based on rules defined in Mutating & Validation webhook configuration
@@ -276,6 +282,7 @@ func main() {
 		rWebhookWatcher,
 		cleanUp,
 		log.Log.WithName("WebhookServer"),
+		openAPIController,
 	)
 	if err != nil {
 		setupLog.Error(err, "Failed to create webhook server")

pkg/kyverno/apply/command.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/nirmata/kyverno/pkg/utils"
 
+	"github.com/nirmata/kyverno/pkg/openapi"
+
 	"github.com/nirmata/kyverno/pkg/kyverno/sanitizedError"
 
 	policy2 "github.com/nirmata/kyverno/pkg/policy"
@@ -69,8 +71,13 @@ func Command() *cobra.Command {
 				}
 			}
 
+			openAPIController, err := openapi.NewOpenAPIController()
+			if err != nil {
+				return err
+			}
+
 			for _, policy := range policies {
-				err := policy2.Validate(utils.MarshalPolicy(*policy), nil, true)
+				err := policy2.Validate(utils.MarshalPolicy(*policy), nil, true, openAPIController)
 				if err != nil {
 					return sanitizedError.New(fmt.Sprintf("Policy %v is not valid", policy.Name))
 				}
@@ -84,7 +91,7 @@ func Command() *cobra.Command {
 				}
 			}
 
-			resources, err := getResources(policies, resourcePaths, dClient)
+			resources, err := getResources(policies, resourcePaths, dClient, openAPIController)
 			if err != nil {
 				return sanitizedError.New(fmt.Errorf("Issues fetching resources").Error())
 			}
@@ -112,7 +119,7 @@ func Command() *cobra.Command {
 	return cmd
 }
 
-func getResources(policies []*v1.ClusterPolicy, resourcePaths []string, dClient discovery.CachedDiscoveryInterface) ([]*unstructured.Unstructured, error) {
+func getResources(policies []*v1.ClusterPolicy, resourcePaths []string, dClient discovery.CachedDiscoveryInterface, openAPIController *openapi.Controller) ([]*unstructured.Unstructured, error) {
 	var resources []*unstructured.Unstructured
 	var err error
 
@@ -131,7 +138,7 @@ func getResources(policies []*v1.ClusterPolicy, resourcePaths []string, dClient
 			resourceTypes = append(resourceTypes, kind)
 		}
 
-		resources, err = getResourcesOfTypeFromCluster(resourceTypes, dClient)
+		resources, err = getResourcesOfTypeFromCluster(resourceTypes, dClient, openAPIController)
 		if err != nil {
 			return nil, err
 		}
@@ -149,11 +156,12 @@ func getResources(policies []*v1.ClusterPolicy, resourcePaths []string, dClient
 	return resources, nil
 }
 
-func getResourcesOfTypeFromCluster(resourceTypes []string, dClient discovery.CachedDiscoveryInterface) ([]*unstructured.Unstructured, error) {
+func getResourcesOfTypeFromCluster(resourceTypes []string, dClient discovery.CachedDiscoveryInterface, openAPIController *openapi.Controller) ([]*unstructured.Unstructured, error) {
 	var resources []*unstructured.Unstructured
 
 	for _, kind := range resourceTypes {
-		endpoint, err := getListEndpointForKind(kind)
+		// TODO use lister interface
+		endpoint, err := getListEndpointForKind(kind, openAPIController)
 		if err != nil {
 			return nil, err
 		}

pkg/kyverno/apply/helper.go

@@ -7,9 +7,9 @@ import (
 	"github.com/nirmata/kyverno/pkg/openapi"
 )
 
-func getListEndpointForKind(kind string) (string, error) {
+func getListEndpointForKind(kind string, openAPIController *openapi.Controller) (string, error) {
 
-	definitionName := openapi.GetDefinitionNameFromKind(kind)
+	definitionName := openAPIController.GetDefinitionNameFromKind(kind)
 	definitionNameWithoutPrefix := strings.Replace(definitionName, "io.k8s.", "", -1)
 
 	parts := strings.Split(definitionNameWithoutPrefix, ".")

pkg/kyverno/validate/command.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/nirmata/kyverno/pkg/utils"
 
+	"github.com/nirmata/kyverno/pkg/openapi"
+
 	"github.com/nirmata/kyverno/pkg/kyverno/sanitizedError"
 
 	policyvalidate "github.com/nirmata/kyverno/pkg/policy"
@@ -43,8 +45,13 @@ func Command() *cobra.Command {
 				}
 			}
 
+			openAPIController, err := openapi.NewOpenAPIController()
+			if err != nil {
+				return err
+			}
+
 			for _, policy := range policies {
-				err = policyvalidate.Validate(utils.MarshalPolicy(*policy), nil, true)
+				err = policyvalidate.Validate(utils.MarshalPolicy(*policy), nil, true, openAPIController)
 				if err != nil {
 					fmt.Println("Policy " + policy.Name + " is invalid")
 				} else {

pkg/openapi/crdSync.go

@@ -2,6 +2,7 @@ package openapi
 
 import (
 	"encoding/json"
+	"fmt"
 	"time"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -17,25 +18,18 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
-type crdDefinition struct {
-	Spec struct {
-		Names struct {
-			Kind string `json:"kind"`
-		} `json:"names"`
-		Versions []struct {
-			Schema struct {
-				OpenAPIV3Schema interface{} `json:"openAPIV3Schema"`
-			} `json:"schema"`
-		} `json:"versions"`
-	} `json:"spec"`
-}
-
 type crdSync struct {
 	client     *client.Client
+	controller *Controller
 }
 
-func NewCRDSync(client *client.Client) *crdSync {
+func NewCRDSync(client *client.Client, controller *Controller) *crdSync {
+	if controller == nil {
+		panic(fmt.Errorf("nil controller sent into crd sync"))
+	}
+
 	return &crdSync{
+		controller: controller,
 		client:     client,
 	}
 }
@@ -46,7 +40,7 @@ func (c *crdSync) Run(workers int, stopCh <-chan struct{}) {
 		log.Log.Error(err, "cannot get openapi schema")
 	}
 
-	err = useOpenApiDocument(newDoc)
+	err = c.controller.useOpenApiDocument(newDoc)
 	if err != nil {
 		log.Log.Error(err, "Could not set custom OpenApi document")
 	}
@@ -66,27 +60,39 @@ func (c *crdSync) sync() {
 		return
 	}
 
-	openApiGlobalState.mutex.Lock()
+	c.controller.mutex.Lock()
-	defer openApiGlobalState.mutex.Unlock()
+	defer c.controller.mutex.Unlock()
 
-	deleteCRDFromPreviousSync()
+	c.controller.deleteCRDFromPreviousSync()
 
 	for _, crd := range crds.Items {
-		parseCRD(crd)
+		c.controller.parseCRD(crd)
 	}
 }
 
-func deleteCRDFromPreviousSync() {
+func (o *Controller) deleteCRDFromPreviousSync() {
-	for _, crd := range openApiGlobalState.crdList {
+	for _, crd := range o.crdList {
-		delete(openApiGlobalState.kindToDefinitionName, crd)
+		delete(o.kindToDefinitionName, crd)
-		delete(openApiGlobalState.definitions, crd)
+		delete(o.definitions, crd)
 	}
 
-	openApiGlobalState.crdList = []string{}
+	o.crdList = []string{}
 }
 
-func parseCRD(crd unstructured.Unstructured) {
+func (o *Controller) parseCRD(crd unstructured.Unstructured) {
-	var crdDefinition crdDefinition
+	var crdDefinition struct {
+		Spec struct {
+			Names struct {
+				Kind string `json:"kind"`
+			} `json:"names"`
+			Versions []struct {
+				Schema struct {
+					OpenAPIV3Schema interface{} `json:"openAPIV3Schema"`
+				} `json:"schema"`
+			} `json:"versions"`
+		} `json:"spec"`
+	}
+
 	crdRaw, _ := json.Marshal(crd.Object)
 	_ = json.Unmarshal(crdRaw, &crdDefinition)
 
@@ -107,10 +113,10 @@ func parseCRD(crd unstructured.Unstructured) {
 		return
 	}
 
-	openApiGlobalState.crdList = append(openApiGlobalState.crdList, crdName)
+	o.crdList = append(o.crdList, crdName)
 
-	openApiGlobalState.kindToDefinitionName[crdName] = crdName
+	o.kindToDefinitionName[crdName] = crdName
-	openApiGlobalState.definitions[crdName] = parsedSchema
+	o.definitions[crdName] = parsedSchema
 }
 
 // addingDefaultFieldsToSchema will add any default missing fields like apiVersion, metadata

pkg/openapi/validation.go

@@ -25,7 +25,7 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-var openApiGlobalState struct {
+type Controller struct {
 	mutex                sync.RWMutex
 	document             *openapi_v2.Document
 	definitions          map[string]*openapi_v2.Schema
@@ -34,21 +34,25 @@ var openApiGlobalState struct {
 	models               proto.Models
 }
 
-func init() {
+func NewOpenAPIController() (*Controller, error) {
+	controller := &Controller{}
+
 	defaultDoc, err := getSchemaDocument()
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
 
-	err = useOpenApiDocument(defaultDoc)
+	err = controller.useOpenApiDocument(defaultDoc)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
+
+	return controller, nil
 }
 
-func ValidatePolicyFields(policyRaw []byte) error {
+func (o *Controller) ValidatePolicyFields(policyRaw []byte) error {
-	openApiGlobalState.mutex.RLock()
+	o.mutex.RLock()
-	defer openApiGlobalState.mutex.RUnlock()
+	defer o.mutex.RUnlock()
 
 	var policy v1.ClusterPolicy
 	err := json.Unmarshal(policyRaw, &policy)
@@ -61,24 +65,24 @@ func ValidatePolicyFields(policyRaw []byte) error {
 		return err
 	}
 
-	err = ValidateResource(*policyUnst.DeepCopy(), "ClusterPolicy")
+	err = o.ValidateResource(*policyUnst.DeepCopy(), "ClusterPolicy")
 	if err != nil {
 		return err
 	}
 
-	return validatePolicyMutation(policy)
+	return o.validatePolicyMutation(policy)
 }
 
-func ValidateResource(patchedResource unstructured.Unstructured, kind string) error {
+func (o *Controller) ValidateResource(patchedResource unstructured.Unstructured, kind string) error {
-	openApiGlobalState.mutex.RLock()
+	o.mutex.RLock()
-	defer openApiGlobalState.mutex.RUnlock()
+	defer o.mutex.RUnlock()
 	var err error
 
-	kind = openApiGlobalState.kindToDefinitionName[kind]
+	kind = o.kindToDefinitionName[kind]
-	schema := openApiGlobalState.models.LookupModel(kind)
+	schema := o.models.LookupModel(kind)
 	if schema == nil {
 		// Check if kind is a CRD
-		schema, err = getSchemaFromDefinitions(kind)
+		schema, err = o.getSchemaFromDefinitions(kind)
 		if err != nil || schema == nil {
 			return fmt.Errorf("pre-validation: couldn't find model %s", kind)
 		}
@@ -97,13 +101,13 @@ func ValidateResource(patchedResource unstructured.Unstructured, kind string) er
 	return nil
 }
 
-func GetDefinitionNameFromKind(kind string) string {
+func (o *Controller) GetDefinitionNameFromKind(kind string) string {
-	openApiGlobalState.mutex.RLock()
+	o.mutex.RLock()
-	defer openApiGlobalState.mutex.RUnlock()
+	defer o.mutex.RUnlock()
-	return openApiGlobalState.kindToDefinitionName[kind]
+	return o.kindToDefinitionName[kind]
 }
 
-func validatePolicyMutation(policy v1.ClusterPolicy) error {
+func (o *Controller) validatePolicyMutation(policy v1.ClusterPolicy) error {
 	var kindToRules = make(map[string][]v1.Rule)
 	for _, rule := range policy.Spec.Rules {
 		if rule.HasMutate() {
@@ -116,7 +120,7 @@ func validatePolicyMutation(policy v1.ClusterPolicy) error {
 	for kind, rules := range kindToRules {
 		newPolicy := *policy.DeepCopy()
 		newPolicy.Spec.Rules = rules
-		resource, _ := generateEmptyResource(openApiGlobalState.definitions[openApiGlobalState.kindToDefinitionName[kind]]).(map[string]interface{})
+		resource, _ := o.generateEmptyResource(o.definitions[o.kindToDefinitionName[kind]]).(map[string]interface{})
 		if resource == nil {
 			log.Log.V(4).Info(fmt.Sprintf("Cannot Validate policy: openApi definition now found for %v", kind))
 			return nil
@@ -128,7 +132,7 @@ func validatePolicyMutation(policy v1.ClusterPolicy) error {
 		if err != nil {
 			return err
 		}
-		err = ValidateResource(*patchedResource.DeepCopy(), kind)
+		err = o.ValidateResource(*patchedResource.DeepCopy(), kind)
 		if err != nil {
 			return err
 		}
@@ -137,22 +141,22 @@ func validatePolicyMutation(policy v1.ClusterPolicy) error {
 	return nil
 }
 
-func useOpenApiDocument(customDoc *openapi_v2.Document) error {
+func (o *Controller) useOpenApiDocument(customDoc *openapi_v2.Document) error {
-	openApiGlobalState.mutex.Lock()
+	o.mutex.Lock()
-	defer openApiGlobalState.mutex.Unlock()
+	defer o.mutex.Unlock()
 
-	openApiGlobalState.document = customDoc
+	o.document = customDoc
 
-	openApiGlobalState.definitions = make(map[string]*openapi_v2.Schema)
+	o.definitions = make(map[string]*openapi_v2.Schema)
-	openApiGlobalState.kindToDefinitionName = make(map[string]string)
+	o.kindToDefinitionName = make(map[string]string)
-	for _, definition := range openApiGlobalState.document.GetDefinitions().AdditionalProperties {
+	for _, definition := range o.document.GetDefinitions().AdditionalProperties {
-		openApiGlobalState.definitions[definition.GetName()] = definition.GetValue()
+		o.definitions[definition.GetName()] = definition.GetValue()
 		path := strings.Split(definition.GetName(), ".")
-		openApiGlobalState.kindToDefinitionName[path[len(path)-1]] = definition.GetName()
+		o.kindToDefinitionName[path[len(path)-1]] = definition.GetName()
 	}
 
 	var err error
-	openApiGlobalState.models, err = proto.NewOpenAPIData(openApiGlobalState.document)
+	o.models, err = proto.NewOpenAPIData(o.document)
 	if err != nil {
 		return err
 	}
@@ -171,13 +175,13 @@ func getSchemaDocument() (*openapi_v2.Document, error) {
 }
 
 // For crd, we do not store definition in document
-func getSchemaFromDefinitions(kind string) (proto.Schema, error) {
+func (o *Controller) getSchemaFromDefinitions(kind string) (proto.Schema, error) {
 	if kind == "" {
 		return nil, errors.New("invalid kind")
 	}
 
 	path := proto.NewPath(kind)
-	definition := openApiGlobalState.definitions[kind]
+	definition := o.definitions[kind]
 	if definition == nil {
 		return nil, errors.New("could not find definition")
 	}
@@ -186,17 +190,17 @@ func getSchemaFromDefinitions(kind string) (proto.Schema, error) {
 	// normal definitions from existing schema such as
 	// `metadata` - this maybe a breaking change.
 	// Removing this may cause policy validate to stop working
-	existingDefinitions, _ := openApiGlobalState.models.(*proto.Definitions)
+	existingDefinitions, _ := o.models.(*proto.Definitions)
 
 	return (existingDefinitions).ParseSchema(definition, &path)
 }
 
-func generateEmptyResource(kindSchema *openapi_v2.Schema) interface{} {
+func (o *Controller) generateEmptyResource(kindSchema *openapi_v2.Schema) interface{} {
 
 	types := kindSchema.GetType().GetValue()
 
 	if kindSchema.GetXRef() != "" {
-		return generateEmptyResource(openApiGlobalState.definitions[strings.TrimPrefix(kindSchema.GetXRef(), "#/definitions/")])
+		return o.generateEmptyResource(o.definitions[strings.TrimPrefix(kindSchema.GetXRef(), "#/definitions/")])
 	}
 
 	if len(types) != 1 {
@@ -220,7 +224,7 @@ func generateEmptyResource(kindSchema *openapi_v2.Schema) interface{} {
 		wg.Add(len(properties))
 		for _, property := range properties {
 			go func(property *openapi_v2.NamedSchema) {
-				prop := generateEmptyResource(property.GetValue())
+				prop := o.generateEmptyResource(property.GetValue())
 				mutex.Lock()
 				props[property.GetName()] = prop
 				mutex.Unlock()
@@ -232,7 +236,7 @@ func generateEmptyResource(kindSchema *openapi_v2.Schema) interface{} {
 	case "array":
 		var array []interface{}
 		for _, schema := range kindSchema.GetItems().GetSchema() {
-			array = append(array, generateEmptyResource(schema))
+			array = append(array, o.generateEmptyResource(schema))
 		}
 		return array
 	case "string":

pkg/openapi/validation_test.go

@@ -47,12 +47,14 @@ func Test_ValidateMutationPolicy(t *testing.T) {
 		},
 	}
 
+	o, _ := NewOpenAPIController()
+
 	for i, tc := range tcs {
 		policy := v1.ClusterPolicy{}
 		_ = json.Unmarshal(tc.policy, &policy)
 
 		var errMessage string
-		err := validatePolicyMutation(policy)
+		err := o.validatePolicyMutation(policy)
 		if err != nil {
 			errMessage = err.Error()
 		}

pkg/policy/validate.go

@@ -18,7 +18,7 @@ import (
 // Validate does some initial check to verify some conditions
 // - One operation per rule
 // - ResourceDescription mandatory checks
-func Validate(policyRaw []byte, client *dclient.Client, mock bool) error {
+func Validate(policyRaw []byte, client *dclient.Client, mock bool, openAPIController *openapi.Controller) error {
 	var p kyverno.ClusterPolicy
 	err := json.Unmarshal(policyRaw, &p)
 	if err != nil {
@@ -75,7 +75,7 @@ func Validate(policyRaw []byte, client *dclient.Client, mock bool) error {
 		}
 	}
 
-	if err := openapi.ValidatePolicyFields(policyRaw); err != nil {
+	if err := openAPIController.ValidatePolicyFields(policyRaw); err != nil {
 		return err
 	}
 

pkg/policy/validate_test.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"testing"
 
+	"github.com/nirmata/kyverno/pkg/openapi"
+
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	"gotest.tools/assert"
 )
@@ -369,7 +371,9 @@ func Test_Validate_Policy(t *testing.T) {
 		}
 	 }`)
 
-	err := Validate(rawPolicy, nil, true)
+	openAPIController, _ := openapi.NewOpenAPIController()
+
+	err := Validate(rawPolicy, nil, true, openAPIController)
 	assert.NilError(t, err)
 }
 
@@ -511,7 +515,8 @@ func Test_Validate_ErrorFormat(t *testing.T) {
 	 }
 	`)
 
-	err := Validate(rawPolicy, nil, true)
+	openAPIController, _ := openapi.NewOpenAPIController()
+	err := Validate(rawPolicy, nil, true, openAPIController)
 	assert.Assert(t, err != nil)
 }
 

pkg/webhooks/mutation.go

@@ -5,8 +5,6 @@ import (
 	"sort"
 	"time"
 
-	"github.com/nirmata/kyverno/pkg/openapi"
-
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	"github.com/nirmata/kyverno/pkg/engine"
@@ -67,7 +65,7 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou
 			logger.V(4).Info("failed to apply policy", "policy", policy.Name)
 			continue
 		}
-		err := openapi.ValidateResource(*engineResponse.PatchedResource.DeepCopy(), engineResponse.PatchedResource.GetKind())
+		err := ws.openAPIController.ValidateResource(*engineResponse.PatchedResource.DeepCopy(), engineResponse.PatchedResource.GetKind())
 		if err != nil {
 			logger.Error(err, "failed to validate resource")
 			continue

pkg/webhooks/policyvalidation.go

@@ -11,7 +11,7 @@ import (
 //HandlePolicyValidation performs the validation check on policy resource
 func (ws *WebhookServer) handlePolicyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
 	//TODO: can this happen? wont this be picked by OpenAPI spec schema ?
-	if err := policyvalidate.Validate(request.Object.Raw, ws.client, false); err != nil {
+	if err := policyvalidate.Validate(request.Object.Raw, ws.client, false, ws.openAPIController); err != nil {
 		return &v1beta1.AdmissionResponse{
 			Allowed: false,
 			Result: &metav1.Status{

pkg/webhooks/server.go

@@ -10,6 +10,8 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/nirmata/kyverno/pkg/openapi"
+
 	"github.com/go-logr/logr"
 	"github.com/nirmata/kyverno/pkg/checker"
 	kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned"
@@ -70,6 +72,7 @@ type WebhookServer struct {
 	grGenerator            *generate.Generator
 	resourceWebhookWatcher *webhookconfig.ResourceWebhookRegister
 	log                    logr.Logger
+	openAPIController      *openapi.Controller
 }
 
 // NewWebhookServer creates new instance of WebhookServer accordingly to given configuration
@@ -91,6 +94,7 @@ func NewWebhookServer(
 	resourceWebhookWatcher *webhookconfig.ResourceWebhookRegister,
 	cleanUp chan<- struct{},
 	log logr.Logger,
+	openAPIController *openapi.Controller,
 ) (*WebhookServer, error) {
 
 	if tlsPair == nil {
@@ -124,6 +128,7 @@ func NewWebhookServer(
 		grGenerator:               grGenerator,
 		resourceWebhookWatcher:    resourceWebhookWatcher,
 		log:                       log,
+		openAPIController:         openAPIController,
 	}
 	mux := http.NewServeMux()
 	mux.HandleFunc(config.MutatingWebhookServicePath, ws.handlerFunc(ws.handleMutateAdmissionRequest, true))