From 0d18ae303bb2d9fead1c10be3f85c301d7175966 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 29 Nov 2024 10:41:55 +0100 Subject: [PATCH 1/2] apiextensions: add pkg/test with CEL unit test helpers Signed-off-by: Dr. Stefan Schimanski --- .../apiextensions-apiserver/pkg/test/cel.go | 150 +++++++++ .../pkg/test/example/apiexports_crd.yaml | 293 ++++++++++++++++++ .../pkg/test/example/apiexports_test.go | 293 ++++++++++++++++++ .../pkg/test/pattern.go | 117 +++++++ 4 files changed, 853 insertions(+) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_crd.yaml create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go new file mode 100644 index 00000000000..4ef64c18ba4 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go @@ -0,0 +1,150 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "context" + "fmt" + "os" + "testing" + + "github.com/stretchr/testify/require" + + "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/cel" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apimachinery/pkg/util/yaml" + celconfig "k8s.io/apiserver/pkg/apis/cel" +) + +// FieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns +// a validator func for testing against samples. +func FieldValidatorsFromFile(t *testing.T, crdFilePath string) (validatorsByVersionByJSONPath map[string]map[string]CELValidateFunc) { + data, err := os.ReadFile(crdFilePath) + require.NoError(t, err) + + var crd apiextensionsv1.CustomResourceDefinition + err = yaml.Unmarshal(data, &crd) + require.NoError(t, err) + + return FieldValidators(t, &crd) +} + +// FieldValidators extracts the CEL validators by version and JSONPath from a CRD and returns +// a validator func for testing against samples. +func FieldValidators(t *testing.T, crd *apiextensionsv1.CustomResourceDefinition) (validatorsByVersionByJSONPath map[string]map[string]CELValidateFunc) { + ret := map[string]map[string]CELValidateFunc{} + for _, v := range crd.Spec.Versions { + var internalSchema apiextensions.JSONSchemaProps + err := apiextensionsv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v.Schema.OpenAPIV3Schema, &internalSchema, nil) + require.NoError(t, err, "failed to convert JSONSchemaProps for version %s: %v", v.Name, err) + structuralSchema, err := schema.NewStructural(&internalSchema) + require.NoError(t, err, "failed to create StructuralSchema for version %s: %v", v.Name, err) + + versionVals, err := findCEL(t, structuralSchema, true, field.NewPath("openAPIV3Schema")) + require.NoError(t, err, "failed to find CEL for version %s: %v", v.Name, err) + ret[v.Name] = versionVals + } + + return ret +} + +// VersionValidatorsFromFile extracts the CEL validators by version from a CRD file and returns +// a validator func for testing against samples. +func VersionValidatorsFromFile(t *testing.T, crdFilePath string) map[string]CELValidateFunc { + data, err := os.ReadFile(crdFilePath) + require.NoError(t, err) + + var crd apiextensionsv1.CustomResourceDefinition + err = yaml.Unmarshal(data, &crd) + require.NoError(t, err) + + ret := map[string]CELValidateFunc{} + for _, v := range crd.Spec.Versions { + var internalSchema apiextensions.JSONSchemaProps + err := apiextensionsv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v.Schema.OpenAPIV3Schema, &internalSchema, nil) + require.NoError(t, err, "failed to convert JSONSchemaProps for version %s: %v", v.Name, err) + structuralSchema, err := schema.NewStructural(&internalSchema) + require.NoError(t, err, "failed to create StructuralSchema for version %s: %v", v.Name, err) + ret[v.Name] = func(obj, old interface{}) field.ErrorList { + errs, _ := cel.NewValidator(structuralSchema, true, celconfig.RuntimeCELCostBudget).Validate(context.TODO(), nil, structuralSchema, obj, old, celconfig.PerCallLimit) + return errs + } + } + + return ret +} + +// VersionValidatorFromFile extracts the CEL validators for a given version from a CRD file and returns +// a validator func for testing against samples. +func VersionValidatorFromFile(t *testing.T, crdFilePath string, version string) (CELValidateFunc, error) { + vals := VersionValidatorsFromFile(t, crdFilePath) + if val, ok := vals[version]; ok { + return val, nil + } + return nil, fmt.Errorf("version %s not found", version) +} + +// CELValidateFunc tests a sample object against a CEL validator. +type CELValidateFunc func(obj, old interface{}) field.ErrorList + +func findCEL(t *testing.T, s *schema.Structural, root bool, pth *field.Path) (map[string]CELValidateFunc, error) { + ret := map[string]CELValidateFunc{} + + if len(s.XValidations) > 0 { + s := *s + pth := *pth + ret[pth.String()] = func(obj, old interface{}) field.ErrorList { + errs, _ := cel.NewValidator(&s, root, celconfig.RuntimeCELCostBudget).Validate(context.TODO(), &pth, &s, obj, old, celconfig.PerCallLimit) + return errs + } + } + + for k, v := range s.Properties { + v := v + sub, err := findCEL(t, &v, false, pth.Child("properties").Child(k)) + if err != nil { + return nil, err + } + + for pth, val := range sub { + ret[pth] = val + } + } + if s.Items != nil { + sub, err := findCEL(t, s.Items, false, pth.Child("items")) + if err != nil { + return nil, err + } + for pth, val := range sub { + ret[pth] = val + } + } + if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil { + sub, err := findCEL(t, s.AdditionalProperties.Structural, false, pth.Child("additionalProperties")) + if err != nil { + return nil, err + } + for pth, val := range sub { + ret[pth] = val + } + } + + return ret, nil +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_crd.yaml b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_crd.yaml new file mode 100644 index 00000000000..edddba53f3e --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_crd.yaml @@ -0,0 +1,293 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.15.0 + name: apiexports.apis.kcp.io +spec: + group: apis.kcp.io + names: + categories: + - kcp + kind: APIExport + listKind: APIExportList + plural: apiexports + singular: apiexport + scope: Cluster + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + - jsonPath: .status.conditions[?(@.type=="VirtualWorkspaceURLsReady")].status + name: Ready + type: string + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + APIExport registers an API and implementation to allow consumption by others + through APIBindings. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: Spec holds the desired state. + properties: + identity: + description: |- + identity points to a secret that contains the API identity in the 'key' file. + The API identity determines an unique etcd prefix for objects stored via this + APIExport. + + + Different APIExport in a workspace can share a common identity, or have different + ones. The identity (the secret) can also be transferred to another workspace + when the APIExport is moved. + + + The identity is a secret of the API provider. The APIBindings referencing this APIExport + will store a derived, non-sensitive value of this identity. + + + The identity of an APIExport cannot be changed. A derived, non-sensitive value of + the identity key is stored in the APIExport status and this value is immutable. + + + The identity is defaulted. A secret with the name of the APIExport is automatically + created. + properties: + secretRef: + description: secretRef is a reference to a secret that contains + the API identity in the 'key' file. + properties: + name: + description: name is unique within a namespace to reference + a secret resource. + type: string + namespace: + description: namespace defines the space within which the + secret name must be unique. + type: string + type: object + x-kubernetes-map-type: atomic + type: object + latestResourceSchemas: + description: |- + latestResourceSchemas records the latest APIResourceSchemas that are exposed + with this APIExport. + + + The schemas can be changed in the life-cycle of the APIExport. These changes + have no effect on existing APIBindings, but only on newly bound ones. + + + For updating existing APIBindings, use an APIDeployment keeping bound + workspaces up-to-date. + items: + type: string + type: array + x-kubernetes-list-type: set + maximalPermissionPolicy: + description: |- + maximalPermissionPolicy will allow for a service provider to set an upper bound on what is allowed + for a consumer of this API. If the policy is not set, no upper bound is applied, + i.e the consuming users can do whatever the user workspace allows the user to do. + + + The policy consists of RBAC (Cluster)Roles and (Cluster)Bindings. A request of a user in + a workspace that binds to this APIExport via an APIBinding is additionally checked against + these rules, with the user name and the groups prefixed with `apis.kcp.io:binding:`. + + + For example: assume a user `adam` with groups `system:authenticated` and `a-team` binds to + this APIExport in another workspace root:org:ws. Then a request in that workspace + against a resource of this APIExport is authorized as every other request in that workspace, + but in addition the RBAC policy here in the APIExport workspace has to grant access to the + user `apis.kcp.io:binding:adam` with the groups `apis.kcp.io:binding:system:authenticated` + and `apis.kcp.io:binding:a-team`. + oneOf: + - required: + - local + properties: + local: + description: local is the policy that is defined in same workspace + as the API Export. + type: object + type: object + permissionClaims: + description: |- + permissionClaims make resources available in APIExport's virtual workspace that are not part + of the actual APIExport resources. + + + PermissionClaims are optional and should be the least access necessary to complete the functions + that the service provider needs. Access is asked for on a GroupResource + identity basis. + + + PermissionClaims must be accepted by the user's explicit acknowledgement. Hence, when claims + change, the respecting objects are not visible immediately. + + + PermissionClaims overlapping with the APIExport resources are ignored. + items: + description: |- + PermissionClaim identifies an object by GR and identity hash. + Its purpose is to determine the added permissions that a service provider may + request and that a consumer may accept and allow the service provider access to. + properties: + all: + description: |- + all claims all resources for the given group/resource. + This is mutually exclusive with resourceSelector. + type: boolean + group: + default: "" + description: |- + group is the name of an API group. + For core groups this is the empty string '""'. + pattern: ^(|[a-z0-9]([-a-z0-9]*[a-z0-9](\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)?)$ + type: string + identityHash: + description: |- + This is the identity for a given APIExport that the APIResourceSchema belongs to. + The hash can be found on APIExport and APIResourceSchema's status. + It will be empty for core types. + Note that one must look this up for a particular KCP instance. + type: string + resource: + description: |- + resource is the name of the resource. + Note: it is worth noting that you can not ask for permissions for resource provided by a CRD + not provided by an api export. + pattern: ^[a-z][-a-z0-9]*[a-z0-9]$ + type: string + resourceSelector: + description: resourceSelector is a list of claimed resource + selectors. + items: + properties: + name: + description: |- + name of an object within a claimed group/resource. + It matches the metadata.name field of the underlying object. + If namespace is unset, all objects matching that name will be claimed. + maxLength: 253 + minLength: 1 + pattern: ^([a-z0-9][-a-z0-9_.]*)?[a-z0-9]$ + type: string + namespace: + description: |- + namespace containing the named object. Matches metadata.namespace field. + If "name" is unset, all objects from the namespace are being claimed. + minLength: 1 + type: string + type: object + x-kubernetes-validations: + - message: at least one field must be set + rule: has(self.__namespace__) || has(self.name) + type: array + required: + - resource + type: object + x-kubernetes-validations: + - message: either "all" or "resourceSelector" must be set + rule: (has(self.all) && self.all) != (has(self.resourceSelector) + && size(self.resourceSelector) > 0) + type: array + x-kubernetes-list-map-keys: + - group + - resource + x-kubernetes-list-type: map + type: object + status: + description: Status communicates the observed state. + properties: + conditions: + description: conditions is a list of conditions that apply to the + APIExport. + items: + description: Condition defines an observation of a object operational + state. + properties: + lastTransitionTime: + description: |- + Last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when + the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + A human readable message indicating details about the transition. + This field may be empty. + type: string + reason: + description: |- + The reason for the condition's last transition in CamelCase. + The specific API may choose whether or not this field is considered a guaranteed API. + This field may not be empty. + type: string + severity: + description: |- + Severity provides an explicit classification of Reason code, so the users or machines can immediately + understand the current situation and act accordingly. + The Severity field MUST be set only when Status=False. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: |- + Type of condition in CamelCase or in foo.example.com/CamelCase. + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions + can be useful (see .node.status.conditions), the ability to deconflict is important. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array + identityHash: + description: |- + identityHash is the hash of the API identity key of this APIExport. This value + is immutable as soon as it is set. + type: string + virtualWorkspaces: + description: |- + virtualWorkspaces contains all APIExport virtual workspace URLs. + + + Deprecated: use APIExportEndpointSlice.status.endpoints instead + items: + properties: + url: + description: url is an APIExport virtual workspace URL. + minLength: 1 + type: string + required: + - url + type: object + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go new file mode 100644 index 00000000000..5f1a1c249e2 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go @@ -0,0 +1,293 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package example + +import ( + "testing" + + "github.com/stretchr/testify/require" + + apitest "k8s.io/apiextensions-apiserver/pkg/test" +) + +func TestAPIExportPermissionClaimCELValidation(t *testing.T) { + testCases := []struct { + name string + current, old map[string]interface{} + wantErrs []string + }{ + { + name: "nothing is set", + current: map[string]interface{}{}, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "all is true", + current: map[string]interface{}{ + "all": true, + }, + }, + { + name: "all is true, resourceSelector is nil", + current: map[string]interface{}{ + "all": true, + "resourceSelector": nil, + }, + }, + { + name: "all is true, resourceSelector is empty", + current: map[string]interface{}{ + "all": true, + "resourceSelector": []interface{}{}, + }, + }, + { + name: "all is true and resourceSelector is set", + current: map[string]interface{}{ + "all": true, + "resourceSelector": []interface{}{ + map[string]interface{}{"namespace": "foo"}, + }, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "all is unset and resourceSelector is nil", + current: map[string]interface{}{ + "resourceSelector": nil, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "all is unset and resourceSelector is empty", + current: map[string]interface{}{ + "resourceSelector": []interface{}{}, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "resourceSelector is set", + current: map[string]interface{}{ + "resourceSelector": []interface{}{ + map[string]interface{}{"namespace": "foo"}, + }, + }, + }, + { + name: "all is false and resourceSelector is nil", + current: map[string]interface{}{ + "all": false, + "resourceSelector": nil, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "empty resource selector", + current: map[string]interface{}{ + "all": false, + "resourceSelector": []interface{}{}, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items: Invalid value: \"object\": either \"all\" or \"resourceSelector\" must be set", + }, + }, + { + name: "logicalcluster fine with non-empty identityHash", + current: map[string]interface{}{ + "group": "core.kcp.io", + "resource": "logicalclusters", + "identityHash": "abc", + "all": true, + }, + }, + } + + validators := apitest.FieldValidatorsFromFile(t, "apiexports_crd.yaml") + + for _, tc := range testCases { + pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items" + validator, found := validators["v1alpha1"][pth] + require.True(t, found, "failed to find validator for %s", pth) + + t.Run(tc.name, func(t *testing.T) { + errs := validator(tc.current, tc.old) + t.Log(errs) + + if got := len(errs); got != len(tc.wantErrs) { + t.Errorf("expected errors %v, got %v", len(tc.wantErrs), len(errs)) + return + } + + for i := range tc.wantErrs { + got := errs[i].Error() + if got != tc.wantErrs[i] { + t.Errorf("want error %q, got %q", tc.wantErrs[i], got) + } + } + }) + } +} + +func TestResourceSelectorCELValidation(t *testing.T) { + testCases := []struct { + name string + current, old map[string]interface{} + wantErrs []string + }{ + { + name: "none is set", + current: map[string]interface{}{ + "name": nil, + "namespace": nil, + }, + wantErrs: []string{ + "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.resourceSelector.items: Invalid value: \"object\": at least one field must be set", + }, + }, + { + name: "namespace is set", + current: map[string]interface{}{ + "name": nil, + "namespace": "foo", + }, + }, + { + name: "name is set", + current: map[string]interface{}{ + "name": "foo", + "namespace": nil, + }, + }, + { + name: "both name and namespace are set", + current: map[string]interface{}{ + "name": "foo", + "namespace": "bar", + }, + }, + } + + validators := apitest.FieldValidatorsFromFile(t, "apiexports_crd.yaml") + + for _, tc := range testCases { + pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.resourceSelector.items" + validator, found := validators["v1alpha1"][pth] + require.True(t, found, "failed to find validator for %s", pth) + + t.Run(tc.name, func(t *testing.T) { + errs := validator(tc.current, tc.old) + t.Log(errs) + + if got := len(errs); got != len(tc.wantErrs) { + t.Errorf("expected errors %v, got %v", len(tc.wantErrs), len(errs)) + return + } + + for i := range tc.wantErrs { + got := errs[i].Error() + if got != tc.wantErrs[i] { + t.Errorf("want error %q, got %q", tc.wantErrs[i], got) + } + } + }) + } +} + +func TestAPIExportPermissionClaimPattern(t *testing.T) { + testCases := []struct { + name string + value string + wantError string + }{ + { + name: "middle dot", + value: "abc.123", + }, + { + name: "middle dash", + value: "abc-123", + }, + { + name: "mixed dash and dot", + value: "ab-c1.23", + }, + { + name: "dot at the end", + value: "abc123.", + wantError: "pattern mismatch", + }, + { + name: "dot at the beginning", + value: ".abc123", + wantError: "pattern mismatch", + }, + { + name: "dash at the end", + value: "abc123-", + wantError: "pattern mismatch", + }, + { + name: "dash at the beginning", + value: "-abc123", + wantError: "pattern mismatch", + }, + { + name: "uppercase", + value: "ABC", + wantError: "pattern mismatch", + }, + { + name: "invalid exclamation marks", + value: "!!!", + wantError: "pattern mismatch", + }, + { + name: "empty", + value: "", + wantError: "pattern mismatch", + }, + } + + validators := apitest.PatternValidatorsFromFile(t, "apiexports_crd.yaml") + + for _, tc := range testCases { + pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.resourceSelector.items.properties.name" + validator, found := validators["v1alpha1"][pth] + require.True(t, found, "failed to find validator for %s", pth) + + t.Run(tc.name, func(t *testing.T) { + err := validator(tc.value) + got := "" + if err != nil { + got = err.Error() + } + if got != tc.wantError { + t.Errorf("want error %q, got %q", tc.wantError, got) + } + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go new file mode 100644 index 00000000000..7f9796e5599 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go @@ -0,0 +1,117 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "errors" + "os" + "regexp" + "testing" + + "github.com/stretchr/testify/require" + + "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/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/yaml" +) + +// PatternValidatorsFromFile extracts the pattern validators by version and JSONPath from a CRD file and returns +// a validator func for testing against samples. +func PatternValidatorsFromFile(t *testing.T, crdFilePath string) (validatorsByVersionByJSONPath map[string]map[string]PatternValidateFunc) { + data, err := os.ReadFile(crdFilePath) + require.NoError(t, err) + + var crd apiextensionsv1.CustomResourceDefinition + err = yaml.Unmarshal(data, &crd) + require.NoError(t, err) + + return PatternValidators(t, &crd) +} + +// PatternValidators extracts the pattern validators by version and JSONPath from a CRD and returns +// a validator func for testing against samples. +func PatternValidators(t *testing.T, crd *apiextensionsv1.CustomResourceDefinition) (validatorsByVersionByJSONPath map[string]map[string]PatternValidateFunc) { + ret := map[string]map[string]PatternValidateFunc{} + for _, v := range crd.Spec.Versions { + var internalSchema apiextensions.JSONSchemaProps + err := apiextensionsv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v.Schema.OpenAPIV3Schema, &internalSchema, nil) + require.NoError(t, err, "failed to convert JSONSchemaProps for version %s: %v", v.Name, err) + structuralSchema, err := schema.NewStructural(&internalSchema) + require.NoError(t, err, "failed to create StructuralSchema for version %s: %v", v.Name, err) + + versionVals, err := findPattern(t, structuralSchema, field.NewPath("openAPIV3Schema")) + require.NoError(t, err, "failed to find CEL for version %s: %v", v.Name, err) + ret[v.Name] = versionVals + } + + return ret +} + +type PatternValidateFunc func(obj interface{}) error + +func findPattern(t *testing.T, s *schema.Structural, pth *field.Path) (map[string]PatternValidateFunc, error) { + ret := map[string]PatternValidateFunc{} + + if len(s.ValueValidation.Pattern) > 0 { + s := *s + pth := *pth + ret[pth.String()] = func(obj interface{}) error { + p, err := regexp.Compile(s.ValueValidation.Pattern) + if err != nil { + return err + } + if p.MatchString(obj.(string)) { + return nil + } + return errors.New("pattern mismatch") + } + } + + for k, v := range s.Properties { + v := v + sub, err := findPattern(t, &v, pth.Child("properties").Child(k)) + if err != nil { + return nil, err + } + + for pth, val := range sub { + ret[pth] = val + } + } + if s.Items != nil { + sub, err := findPattern(t, s.Items, pth.Child("items")) + if err != nil { + return nil, err + } + for pth, val := range sub { + ret[pth] = val + } + } + if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil { + sub, err := findPattern(t, s.AdditionalProperties.Structural, pth.Child("additionalProperties")) + if err != nil { + return nil, err + } + for pth, val := range sub { + ret[pth] = val + } + } + + return ret, nil +} From 0c1b1e04501e9a61c844152a2736ce926de804e0 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 16 Dec 2024 13:51:06 +0100 Subject: [PATCH 2/2] Address comments Signed-off-by: Dr. Stefan Schimanski --- .../apiextensions-apiserver/pkg/test/cel.go | 11 +++++------ .../pkg/test/example/apiexports_test.go | 7 ++++--- .../apiextensions-apiserver/pkg/test/pattern.go | 15 --------------- 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go index 4ef64c18ba4..f88d2a63663 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/cel.go @@ -33,17 +33,16 @@ import ( celconfig "k8s.io/apiserver/pkg/apis/cel" ) -// FieldValidatorsFromFile extracts the CEL validators by version and JSONPath from a CRD file and returns -// a validator func for testing against samples. -func FieldValidatorsFromFile(t *testing.T, crdFilePath string) (validatorsByVersionByJSONPath map[string]map[string]CELValidateFunc) { - data, err := os.ReadFile(crdFilePath) +// MustLoadManifest loads a CRD from a file and panics on error. +func MustLoadManifest[T any](t *testing.T, pth string) *T { + data, err := os.ReadFile(pth) require.NoError(t, err) - var crd apiextensionsv1.CustomResourceDefinition + var crd T err = yaml.Unmarshal(data, &crd) require.NoError(t, err) - return FieldValidators(t, &crd) + return &crd } // FieldValidators extracts the CEL validators by version and JSONPath from a CRD and returns diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go index 5f1a1c249e2..830ce6fb712 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/example/apiexports_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/require" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apitest "k8s.io/apiextensions-apiserver/pkg/test" ) @@ -126,7 +127,7 @@ func TestAPIExportPermissionClaimCELValidation(t *testing.T) { }, } - validators := apitest.FieldValidatorsFromFile(t, "apiexports_crd.yaml") + validators := apitest.FieldValidators(t, apitest.MustLoadManifest[apiextensionsv1.CustomResourceDefinition](t, "apiexports_crd.yaml")) for _, tc := range testCases { pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items" @@ -191,7 +192,7 @@ func TestResourceSelectorCELValidation(t *testing.T) { }, } - validators := apitest.FieldValidatorsFromFile(t, "apiexports_crd.yaml") + validators := apitest.FieldValidators(t, apitest.MustLoadManifest[apiextensionsv1.CustomResourceDefinition](t, "apiexports_crd.yaml")) for _, tc := range testCases { pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.resourceSelector.items" @@ -272,7 +273,7 @@ func TestAPIExportPermissionClaimPattern(t *testing.T) { }, } - validators := apitest.PatternValidatorsFromFile(t, "apiexports_crd.yaml") + validators := apitest.PatternValidators(t, apitest.MustLoadManifest[apiextensionsv1.CustomResourceDefinition](t, "apiexports_crd.yaml")) for _, tc := range testCases { pth := "openAPIV3Schema.properties.spec.properties.permissionClaims.items.properties.resourceSelector.items.properties.name" diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go index 7f9796e5599..2c4da5f79bf 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/test/pattern.go @@ -18,7 +18,6 @@ package test import ( "errors" - "os" "regexp" "testing" @@ -28,22 +27,8 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apimachinery/pkg/util/validation/field" - "sigs.k8s.io/yaml" ) -// PatternValidatorsFromFile extracts the pattern validators by version and JSONPath from a CRD file and returns -// a validator func for testing against samples. -func PatternValidatorsFromFile(t *testing.T, crdFilePath string) (validatorsByVersionByJSONPath map[string]map[string]PatternValidateFunc) { - data, err := os.ReadFile(crdFilePath) - require.NoError(t, err) - - var crd apiextensionsv1.CustomResourceDefinition - err = yaml.Unmarshal(data, &crd) - require.NoError(t, err) - - return PatternValidators(t, &crd) -} - // PatternValidators extracts the pattern validators by version and JSONPath from a CRD and returns // a validator func for testing against samples. func PatternValidators(t *testing.T, crd *apiextensionsv1.CustomResourceDefinition) (validatorsByVersionByJSONPath map[string]map[string]PatternValidateFunc) {