From 9b38289a845990810112b5fb1e0930375d29cd58 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Thu, 20 Feb 2020 15:09:20 -0800 Subject: [PATCH] remove openapi validation(manual revert) --- cmd/kyverno/main.go | 20 --- pkg/openapi/helper.go | 5 - pkg/openapi/validation.go | 270 --------------------------------- pkg/openapi/validation_test.go | 65 -------- pkg/policy/validate.go | 6 - pkg/webhooks/mutation.go | 6 - 6 files changed, 372 deletions(-) delete mode 100644 pkg/openapi/helper.go delete mode 100644 pkg/openapi/validation.go delete mode 100644 pkg/openapi/validation_test.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index b8768ba0f1..62cefceeb9 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -5,10 +5,6 @@ import ( "flag" "time" - "github.com/nirmata/kyverno/pkg/openapi" - - "k8s.io/client-go/discovery" - "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/checker" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -204,22 +200,6 @@ func main() { glog.Fatalf("Failed registering Admission Webhooks: %v\n", err) } - // OpenAPI document - // Getting openApi document from kubernetes and overriding default openapi document - restClient, err := discovery.NewDiscoveryClientForConfig(clientConfig) - if err != nil { - glog.Fatalf("Could not get rest client to get openApi doc: %v\n", err) - } - - openApiDoc, err := restClient.RESTClient().Get().RequestURI("/openapi/v2").Do().Raw() - if err != nil { - glog.Fatalf("OpenApiDoc request failed: %v\n", err) - } - - if err := openapi.UseCustomOpenApiDocument(openApiDoc); err != nil { - glog.Fatalf("Could not set custom OpenApi document: %v\n", err) - } - // WEBHOOOK // - https server to provide endpoints called based on rules defined in Mutating & Validation webhook configuration // - reports the results based on the response from the policy engine: diff --git a/pkg/openapi/helper.go b/pkg/openapi/helper.go deleted file mode 100644 index b04978afb5..0000000000 --- a/pkg/openapi/helper.go +++ /dev/null @@ -1,5 +0,0 @@ -package openapi - -func GetDefinitionNameFromKind(kind string) string { - return openApiGlobalState.kindToDefinitionName[kind] -} diff --git a/pkg/openapi/validation.go b/pkg/openapi/validation.go deleted file mode 100644 index 0b941720ee..0000000000 --- a/pkg/openapi/validation.go +++ /dev/null @@ -1,270 +0,0 @@ -package openapi - -import ( - "fmt" - "strconv" - "strings" - "sync" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - "github.com/nirmata/kyverno/data" - - "github.com/golang/glog" - - "github.com/nirmata/kyverno/pkg/engine" - "github.com/nirmata/kyverno/pkg/engine/context" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - - openapi_v2 "github.com/googleapis/gnostic/OpenAPIv2" - "github.com/googleapis/gnostic/compiler" - "k8s.io/kube-openapi/pkg/util/proto" - "k8s.io/kube-openapi/pkg/util/proto/validation" - - "gopkg.in/yaml.v2" -) - -var openApiGlobalState struct { - document *openapi_v2.Document - definitions map[string]*openapi_v2.Schema - kindToDefinitionName map[string]string - models proto.Models - isSet bool -} - -func init() { - err := setValidationGlobalState() - if err != nil { - panic(err) - } -} - -func UseCustomOpenApiDocument(customDoc []byte) error { - var spec yaml.MapSlice - err := yaml.Unmarshal(customDoc, &spec) - if err != nil { - return err - } - - openApiGlobalState.document, err = openapi_v2.NewDocument(spec, compiler.NewContext("$root", nil)) - if err != nil { - return err - } - - openApiGlobalState.definitions = make(map[string]*openapi_v2.Schema) - openApiGlobalState.kindToDefinitionName = make(map[string]string) - for _, definition := range openApiGlobalState.document.GetDefinitions().AdditionalProperties { - openApiGlobalState.definitions[definition.GetName()] = definition.GetValue() - path := strings.Split(definition.GetName(), ".") - openApiGlobalState.kindToDefinitionName[path[len(path)-1]] = definition.GetName() - } - - openApiGlobalState.models, err = proto.NewOpenAPIData(openApiGlobalState.document) - if err != nil { - return err - } - - openApiGlobalState.isSet = true - - return nil -} - -func ValidatePolicyMutation(policy v1.ClusterPolicy) error { - if !openApiGlobalState.isSet { - glog.V(4).Info("Cannot Validate policy: Validation global state not set") - return nil - } - - var kindToRules = make(map[string][]v1.Rule) - for _, rule := range policy.Spec.Rules { - if rule.HasMutate() { - rule.MatchResources = v1.MatchResources{ - UserInfo: v1.UserInfo{}, - ResourceDescription: v1.ResourceDescription{ - Kinds: rule.MatchResources.Kinds, - }, - } - rule.ExcludeResources = v1.ExcludeResources{} - for _, kind := range rule.MatchResources.Kinds { - kindToRules[kind] = append(kindToRules[kind], rule) - } - } - } - - for kind, rules := range kindToRules { - newPolicy := policy - newPolicy.Spec.Rules = rules - - resource, _ := generateEmptyResource(openApiGlobalState.definitions[openApiGlobalState.kindToDefinitionName[kind]]).(map[string]interface{}) - newResource := unstructured.Unstructured{Object: resource} - newResource.SetKind(kind) - policyContext := engine.PolicyContext{ - Policy: newPolicy, - NewResource: newResource, - Context: context.NewContext(), - } - resp := engine.Mutate(policyContext) - if len(resp.GetSuccessRules()) != len(rules) { - var errMessages []string - for _, rule := range resp.PolicyResponse.Rules { - if !rule.Success { - errMessages = append(errMessages, fmt.Sprintf("Invalid rule : %v, %v", rule.Name, rule.Message)) - } - } - return fmt.Errorf(strings.Join(errMessages, "\n")) - } - err := ValidateResource(resp.PatchedResource.UnstructuredContent(), kind) - if err != nil { - return err - } - } - - return nil -} - -func ValidateResource(patchedResource interface{}, kind string) error { - if !openApiGlobalState.isSet { - glog.V(4).Info("Cannot Validate resource: Validation global state not set") - return nil - } - - kind = openApiGlobalState.kindToDefinitionName[kind] - - schema := openApiGlobalState.models.LookupModel(kind) - if schema == nil { - return fmt.Errorf("pre-validation: couldn't find model %s", kind) - } - - if errs := validation.ValidateModel(patchedResource, schema, kind); len(errs) > 0 { - var errorMessages []string - for i := range errs { - errorMessages = append(errorMessages, errs[i].Error()) - } - - return fmt.Errorf(strings.Join(errorMessages, "\n\n")) - } - - return nil -} - -func setValidationGlobalState() error { - if !openApiGlobalState.isSet { - var err error - openApiGlobalState.document, err = getSchemaDocument() - if err != nil { - return err - } - - openApiGlobalState.definitions = make(map[string]*openapi_v2.Schema) - openApiGlobalState.kindToDefinitionName = make(map[string]string) - for _, definition := range openApiGlobalState.document.GetDefinitions().AdditionalProperties { - openApiGlobalState.definitions[definition.GetName()] = definition.GetValue() - path := strings.Split(definition.GetName(), ".") - openApiGlobalState.kindToDefinitionName[path[len(path)-1]] = definition.GetName() - } - - for _, path := range openApiGlobalState.document.GetPaths().GetPath() { - path.GetName() - } - - openApiGlobalState.models, err = proto.NewOpenAPIData(openApiGlobalState.document) - if err != nil { - return err - } - - openApiGlobalState.isSet = true - } - return nil -} - -func getSchemaDocument() (*openapi_v2.Document, error) { - var spec yaml.MapSlice - err := yaml.Unmarshal([]byte(data.SwaggerDoc), &spec) - if err != nil { - return nil, err - } - - return openapi_v2.NewDocument(spec, compiler.NewContext("$root", nil)) -} - -func generateEmptyResource(kindSchema *openapi_v2.Schema) interface{} { - - types := kindSchema.GetType().GetValue() - - if kindSchema.GetXRef() != "" { - return generateEmptyResource(openApiGlobalState.definitions[strings.TrimPrefix(kindSchema.GetXRef(), "#/definitions/")]) - } - - if len(types) != 1 { - return nil - } - - switch types[0] { - case "object": - var props = make(map[string]interface{}) - properties := kindSchema.GetProperties().GetAdditionalProperties() - if len(properties) == 0 { - return props - } - - var wg sync.WaitGroup - var mutex sync.Mutex - wg.Add(len(properties)) - for _, property := range properties { - go func(property *openapi_v2.NamedSchema) { - prop := generateEmptyResource(property.GetValue()) - mutex.Lock() - props[property.GetName()] = prop - mutex.Unlock() - wg.Done() - }(property) - } - wg.Wait() - return props - case "array": - var array []interface{} - for _, schema := range kindSchema.GetItems().GetSchema() { - array = append(array, generateEmptyResource(schema)) - } - return array - case "string": - if kindSchema.GetDefault() != nil { - return string(kindSchema.GetDefault().Value.Value) - } - if kindSchema.GetExample() != nil { - return string(kindSchema.GetExample().GetValue().Value) - } - return "" - case "integer": - if kindSchema.GetDefault() != nil { - val, _ := strconv.Atoi(string(kindSchema.GetDefault().Value.Value)) - return int64(val) - } - if kindSchema.GetExample() != nil { - val, _ := strconv.Atoi(string(kindSchema.GetExample().GetValue().Value)) - return int64(val) - } - return int64(0) - case "number": - if kindSchema.GetDefault() != nil { - val, _ := strconv.Atoi(string(kindSchema.GetDefault().Value.Value)) - return int64(val) - } - if kindSchema.GetExample() != nil { - val, _ := strconv.Atoi(string(kindSchema.GetExample().GetValue().Value)) - return int64(val) - } - return int64(0) - case "boolean": - if kindSchema.GetDefault() != nil { - return string(kindSchema.GetDefault().Value.Value) == "true" - } - if kindSchema.GetExample() != nil { - return string(kindSchema.GetExample().GetValue().Value) == "true" - } - return false - } - - return nil -} diff --git a/pkg/openapi/validation_test.go b/pkg/openapi/validation_test.go deleted file mode 100644 index 4c6c8ac2a6..0000000000 --- a/pkg/openapi/validation_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package openapi - -import ( - "encoding/json" - "testing" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" -) - -func Test_ValidateMutationPolicy(t *testing.T) { - err := setValidationGlobalState() - if err != nil { - t.Fatalf("Could not set global state") - } - - tcs := []struct { - description string - policy []byte - errMessage string - }{ - { - description: "Policy with mutating imagePullPolicy Overlay", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-image-pull-policy-2"},"spec":{"rules":[{"name":"set-image-pull-policy-2","match":{"resources":{"kinds":["Pod"]}},"mutate":{"overlay":{"spec":{"containers":[{"(image)":"*","imagePullPolicy":"Always"}]}}}}]}}`), - }, - { - description: "Policy with mutating imagePullPolicy Overlay, field does not exist", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-image-pull-policy-2"},"spec":{"rules":[{"name":"set-image-pull-policy-2","match":{"resources":{"kinds":["Pod"]}},"mutate":{"overlay":{"spec":{"containers":[{"(image)":"*","nonExistantField":"Always"}]}}}}]}}`), - errMessage: `ValidationError(io.k8s.api.core.v1.Pod.spec.containers[0]): unknown field "nonExistantField" in io.k8s.api.core.v1.Container`, - }, - { - description: "Policy with mutating imagePullPolicy Overlay, type of value is different (does not throw error since all numbers are also strings according to swagger)", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-image-pull-policy-2"},"spec":{"rules":[{"name":"set-image-pull-policy-2","match":{"resources":{"kinds":["Pod"]}},"mutate":{"overlay":{"spec":{"containers":[{"(image)":"*","imagePullPolicy":80}]}}}}]}}`), - }, - { - description: "Policy with patches", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"policy-endpoints"},"spec":{"rules":[{"name":"pEP","match":{"resources":{"kinds":["Endpoints"],"selector":{"matchLabels":{"label":"test"}}}},"mutate":{"patches":[{"path":"/subsets/0/ports/0/port","op":"replace","value":9663},{"path":"/metadata/labels/isMutated","op":"add","value":"true"}]}}]}}`), - }, - { - description: "Policy with patches, value converted from number to string", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"policy-endpoints"},"spec":{"rules":[{"name":"pEP","match":{"resources":{"kinds":["Endpoints"],"selector":{"matchLabels":{"label":"test"}}}},"mutate":{"patches":[{"path":"/subsets/0/ports/0/port","op":"replace","value":"9663"},{"path":"/metadata/labels/isMutated","op":"add","value":"true"}]}}]}}`), - errMessage: `ValidationError(io.k8s.api.core.v1.Endpoints.subsets[0].ports[0].port): invalid type for io.k8s.api.core.v1.EndpointPort.port: got "string", expected "integer"`, - }, - { - description: "Policy where boolean is been converted to number", - policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"mutate-pod-disable-automoutingapicred"},"spec":{"rules":[{"name":"pod-disable-automoutingapicred","match":{"resources":{"kinds":["Pod"]}},"mutate":{"overlay":{"spec":{"(serviceAccountName)":"*","automountServiceAccountToken":80}}}}]}}`), - errMessage: `ValidationError(io.k8s.api.core.v1.Pod.spec.automountServiceAccountToken): invalid type for io.k8s.api.core.v1.PodSpec.automountServiceAccountToken: got "integer", expected "boolean"`, - }, - } - - for i, tc := range tcs { - policy := v1.ClusterPolicy{} - _ = json.Unmarshal(tc.policy, &policy) - - var errMessage string - err := ValidatePolicyMutation(policy) - if err != nil { - errMessage = err.Error() - } - - if errMessage != tc.errMessage { - t.Errorf("\nTestcase [%v] failed:\nExpected Error: %v\nGot Error: %v", i+1, tc.errMessage, errMessage) - } - } - -} diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 627437c9f5..b5071a01a3 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -8,8 +8,6 @@ import ( "strconv" "strings" - "github.com/nirmata/kyverno/pkg/openapi" - kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine/anchor" rbacv1 "k8s.io/api/rbac/v1" @@ -73,10 +71,6 @@ func Validate(p kyverno.ClusterPolicy) error { } } - if err := openapi.ValidatePolicyMutation(p); err != nil { - return fmt.Errorf("Failed to validate policy: %v", err) - } - return nil } diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index b081f04ef0..c664c8a03f 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -9,7 +9,6 @@ import ( "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" engineutils "github.com/nirmata/kyverno/pkg/engine/utils" - "github.com/nirmata/kyverno/pkg/openapi" policyctr "github.com/nirmata/kyverno/pkg/policy" "github.com/nirmata/kyverno/pkg/policyviolation" "github.com/nirmata/kyverno/pkg/utils" @@ -102,11 +101,6 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue } - err := openapi.ValidateResource(engineResponse.PatchedResource.UnstructuredContent(), engineResponse.PatchedResource.GetKind()) - if err != nil { - glog.V(4).Infoln(err) - continue - } // gather patches patches = append(patches, engineResponse.GetPatches()...) glog.V(4).Infof("Mutation from policy %s has applied successfully to %s %s/%s", policy.Name, request.Kind.Kind, resource.GetNamespace(), resource.GetName())