github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Allowing direct CEL reserved keyword usage in CRD (#126188)

Browse Source 
* automatically escape reserved keywords for direct usage

* Add reserved keyword support in a ratcheting way, add tests.

---------

Co-authored-by: Wenxue Zhao <ballista01@outlook.com>
This commit is contained in:
Cici Huang
2024-07-23 15:45:20 -07:00
committed by GitHub
parent fa4b8f32ac
commit a48a92c72e
7 changed files with 458 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

329
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
View File

@@ -9012,6 +9012,189 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
requireStructuralSchema: true,
},
},
{
name: "invalid x-kubernetes-validations for escaping (keywords are invalid field names before v1.30)",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "self.__if__ > 0",
},
{
Rule: "self.__namespace__ > 0",
},
{
Rule: "self.if > 0",
},
{
Rule: "self.namespace > 0",
},
{
Rule: "self.as > 0",
},
{
Rule: "self.break > 0",
},
{
Rule: "self.const > 0",
},
{
Rule: "self.continue > 0",
},
{
Rule: "self.else > 0",
},
{
Rule: "self.for > 0",
},
{
Rule: "self.function > 0",
},
{
Rule: "self.import > 0",
},
{
Rule: "self.let > 0",
},
{
Rule: "self.loop > 0",
},
{
Rule: "self.package > 0",
},
{
Rule: "self.return > 0",
},
{
Rule: "self.var > 0",
},
{
Rule: "self.void > 0",
},
{
Rule: "self.while > 0",
},
{
Rule: "self.self > 0",
},
{
Rule: "self.int > 0",
},
{
Rule: "self.true > 0",
},
{
Rule: "self.false > 0",
},
{
Rule: "self.null > 0",
},
{
Rule: "self.in > 0",
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"if": {
Type: "integer",
},
"namespace": {
Type: "integer",
},
"as": {
Type: "integer",
},
"break": {
Type: "integer",
},
"const": {
Type: "integer",
},
"continue": {
Type: "integer",
},
"else": {
Type: "integer",
},
"for": {
Type: "integer",
},
"function": {
Type: "integer",
},
"import": {
Type: "integer",
},
"let": {
Type: "integer",
},
"loop": {
Type: "integer",
},
"package": {
Type: "integer",
},
"return": {
Type: "integer",
},
"var": {
Type: "integer",
},
"void": {
Type: "integer",
},
"while": {
Type: "integer",
},
"self": {
Type: "integer",
},
"int": {
Type: "integer",
},
"true": {
Type: "integer",
},
"false": {
Type: "integer",
},
"null": {
Type: "integer",
},
"in": {
Type: "integer",
},
},
},
},
opts: validationOptions{
requireStructuralSchema: true,
celEnvironmentSet: environment.MustBaseEnvSet(version.MajorMinor(1, 30), true),
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[2].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[3].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[4].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[5].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[6].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[7].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[8].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[9].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[10].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[11].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[12].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[13].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[14].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[15].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[16].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[17].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[18].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[21].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[22].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[23].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[24].rule"),
},
},
{
name: "valid x-kubernetes-validations for escaping",
input: apiextensions.CustomResourceValidation{
@@ -9024,12 +9207,76 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
{
Rule: "self.__namespace__ > 0",
},
{
Rule: "self.if > 0",
},
{
Rule: "self.namespace > 0",
},
{
Rule: "self.as > 0",
},
{
Rule: "self.break > 0",
},
{
Rule: "self.const > 0",
},
{
Rule: "self.continue > 0",
},
{
Rule: "self.else > 0",
},
{
Rule: "self.for > 0",
},
{
Rule: "self.function > 0",
},
{
Rule: "self.import > 0",
},
{
Rule: "self.let > 0",
},
{
Rule: "self.loop > 0",
},
{
Rule: "self.package > 0",
},
{
Rule: "self.return > 0",
},
{
Rule: "self.var > 0",
},
{
Rule: "self.void > 0",
},
{
Rule: "self.while > 0",
},
{
Rule: "self.self > 0",
},
{
Rule: "self.int > 0",
},
// reserved keywords `true`, `false`, `null` and `in` are not supported
{
Rule: "self.true > 0",
},
{
Rule: "self.false > 0",
},
{
Rule: "self.null > 0",
},
{
Rule: "self.in > 0",
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"if": {
@@ -9038,49 +9285,97 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
"namespace": {
Type: "integer",
},
"as": {
Type: "integer",
},
"break": {
Type: "integer",
},
"const": {
Type: "integer",
},
"continue": {
Type: "integer",
},
"else": {
Type: "integer",
},
"for": {
Type: "integer",
},
"function": {
Type: "integer",
},
"import": {
Type: "integer",
},
"let": {
Type: "integer",
},
"loop": {
Type: "integer",
},
"package": {
Type: "integer",
},
"return": {
Type: "integer",
},
"var": {
Type: "integer",
},
"void": {
Type: "integer",
},
"while": {
Type: "integer",
},
"self": {
Type: "integer",
},
"int": {
Type: "integer",
},
"true": {
Type: "integer",
},
"false": {
Type: "integer",
},
"null": {
Type: "integer",
},
"in": {
Type: "integer",
},
},
},
},
opts: validationOptions{
requireStructuralSchema: true,
celEnvironmentSet: environment.MustBaseEnvSet(version.MajorMinor(1, 31), true),
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[21].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[22].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[23].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[24].rule"),
},
},
{
name: "invalid x-kubernetes-validations for escaping",
name: "invalid x-kubernetes-validations for unknown property",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "self.if > 0",
},
{
Rule: "self.namespace > 0",
},
{
Rule: "self.unknownProp > 0",
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"if": {
Type: "integer",
},
"namespace": {
Type: "integer",
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[0].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[1].rule"),
invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[2].rule"),
},
opts: validationOptions{
requireStructuralSchema: true,

22
staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/types_test.go
View File

@@ -22,40 +22,38 @@ import (
"github.com/google/cel-go/cel"
"github.com/google/cel-go/common/types"
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
apiservercel "k8s.io/apiserver/pkg/cel"
)
func TestTypes_RuleTypesFieldMapping(t *testing.T) {
stdEnv, _ := cel.NewEnv()
rt := apiservercel.NewDeclTypeProvider(SchemaDeclType(testSchema(), true).MaybeAssignTypeName("CustomObject"))
nestedFieldType, found := rt.FindFieldType("CustomObject", "nested")
nestedFieldType, found := rt.FindStructFieldType("CustomObject", "nested")
if !found {
t.Fatal("got field not found for 'CustomObject.nested', wanted found")
}
if nestedFieldType.Type.GetMessageType() != "CustomObject.nested" {
if nestedFieldType.Type.DeclaredTypeName() != "CustomObject.nested" {
t.Errorf("got field type %v, wanted mock_template.nested", nestedFieldType.Type)
}
subnameFieldType, found := rt.FindFieldType("CustomObject.nested", "subname")
subnameFieldType, found := rt.FindStructFieldType("CustomObject.nested", "subname")
if !found {
t.Fatal("got field not found for 'CustomObject.nested.subname', wanted found")
}
if subnameFieldType.Type.GetPrimitive() != exprpb.Type_STRING {
if subnameFieldType.Type.TypeName() != "string" {
t.Errorf("got field type %v, wanted string", subnameFieldType.Type)
}
flagsFieldType, found := rt.FindFieldType("CustomObject.nested", "flags")
flagsFieldType, found := rt.FindStructFieldType("CustomObject.nested", "flags")
if !found {
t.Fatal("got field not found for 'CustomObject.nested.flags', wanted found")
}
if flagsFieldType.Type.GetMapType() == nil {
if flagsFieldType.Type.Kind() != types.MapKind {
t.Errorf("got field type %v, wanted map", flagsFieldType.Type)
}
flagFieldType, found := rt.FindFieldType("CustomObject.nested.flags", "my_flag")
flagFieldType, found := rt.FindStructFieldType("CustomObject.nested.flags", "my_flag")
if !found {
t.Fatal("got field not found for 'CustomObject.nested.flags.my_flag', wanted found")
}
if flagFieldType.Type.GetPrimitive() != exprpb.Type_BOOL {
if flagFieldType.Type.Kind() != types.BoolKind {
t.Errorf("got field type %v, wanted bool", flagFieldType.Type)
}
@@ -90,7 +88,7 @@ func TestTypes_RuleTypesFieldMapping(t *testing.T) {
// t.Error("got CEL rule value of nil, wanted non-nil")
//}
opts, err := rt.EnvOptions(stdEnv.TypeProvider())
opts, err := rt.EnvOptions(stdEnv.CELTypeProvider())
if err != nil {
t.Fatal(err)
}
@@ -98,7 +96,7 @@ func TestTypes_RuleTypesFieldMapping(t *testing.T) {
if err != nil {
t.Fatal(err)
}
helloVal := ruleEnv.TypeAdapter().NativeToValue("hello")
helloVal := ruleEnv.CELTypeAdapter().NativeToValue("hello")
if helloVal.Equal(types.String("hello")) != types.True {
t.Errorf("got %v, wanted types.String('hello')", helloVal)
}

42
staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation_test.go
View File

@@ -899,6 +899,48 @@ func TestValidationExpressions(t *testing.T) {
"has(self.embedded.metadata.namespace)": "undefined field 'namespace'",
},
},
{name: "embedded object with usage of reserved keywords",
obj: map[string]interface{}{
"embedded": map[string]interface{}{
"apiVersion": "v1",
"kind": "Pod",
"metadata": map[string]interface{}{
"name": "foo",
"generateName": "pickItForMe",
"namespace": "reserved_keyword_namespace",
},
"spec": map[string]interface{}{
"if": "reserved_keyword_if",
},
},
},
schema: objectTypePtr(map[string]schema.Structural{
"embedded": {
Generic: schema.Generic{Type: "object"},
Extensions: schema.Extensions{
XEmbeddedResource: true,
},
Properties: map[string]schema.Structural{
"kind": stringType,
"apiVersion": stringType,
"metadata": objectType(map[string]schema.Structural{
"name": stringType,
"generateName": stringType,
"namespace": stringType,
}),
"spec": objectType(map[string]schema.Structural{
"if": stringType,
}),
},
},
}),
valid: []string{
"has(self.embedded.metadata.namespace)",
"self.embedded.metadata.namespace == 'reserved_keyword_namespace'",
"has(self.embedded.spec.if)",
"self.embedded.spec.if == 'reserved_keyword_if'",
},
},
{name: "embedded object with preserve unknown",
obj: map[string]interface{}{
"embedded": map[string]interface{}{

5
staging/src/k8s.io/apiserver/pkg/cel/environment/environment.go
View File

@@ -267,7 +267,10 @@ func (e *EnvSet) filterAndBuildOpts(base *cel.Env, compatVer *version.Version, h
if len(declTypes) > 0 {
provider := apiservercel.NewDeclTypeProvider(declTypes...)
providerOpts, err := provider.EnvOptions(base.TypeProvider())
if compatVer.AtLeast(version.MajorMinor(1, 31)) {
provider.SetRecognizeKeywordAsFieldName(true)
}
providerOpts, err := provider.EnvOptions(base.CELTypeProvider())
if err != nil {
return nil, err
}

45
staging/src/k8s.io/apiserver/pkg/cel/environment/environment_test.go
View File

@@ -43,6 +43,15 @@ func TestBaseEnvironment(t *testing.T) {
},
})
// The escaping happens while construct declType hence we use escaped format here directly.
gadgetsType := apiservercel.NewObjectType("Gadget",
map[string]*apiservercel.DeclField{
"__namespace__": {
Name: "__namespace__",
Type: apiservercel.StringType,
},
})
cases := []struct {
name string
typeVersionCombinations []envTypeAndVersion
@@ -251,6 +260,42 @@ func TestBaseEnvironment(t *testing.T) {
},
},
},
{
name: "recognizeKeywordAsFieldName disabled",
typeVersionCombinations: []envTypeAndVersion{
{version.MajorMinor(1, 30), NewExpressions},
// always enabled for StoredExpressions
},
invalidExpressions: []string{"gadget.namespace == 'buzz'"},
activation: map[string]any{"gadget": map[string]any{"namespace": "buzz"}},
opts: []VersionedOptions{
{
IntroducedVersion: version.MajorMinor(1, 28),
DeclTypes: []*apiservercel.DeclType{gadgetsType},
EnvOptions: []cel.EnvOption{
cel.Variable("gadget", cel.ObjectType("Gadget")),
},
},
},
},
{
name: "recognizeKeywordAsFieldName enabled",
typeVersionCombinations: []envTypeAndVersion{
{version.MajorMinor(1, 31), NewExpressions},
{version.MajorMinor(1, 30), StoredExpressions},
},
validExpressions: []string{"gadget.namespace == 'buzz'"},
activation: map[string]any{"gadget": map[string]any{"namespace": "buzz"}},
opts: []VersionedOptions{
{
IntroducedVersion: version.MajorMinor(1, 28),
DeclTypes: []*apiservercel.DeclType{gadgetsType},
EnvOptions: []cel.EnvOption{
cel.Variable("gadget", cel.ObjectType("Gadget")),
},
},
},
},
}
for _, tc := range cases {

74
staging/src/k8s.io/apiserver/pkg/cel/types.go
View File

@@ -27,7 +27,6 @@ import (
"github.com/google/cel-go/common/types/traits"
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
"google.golang.org/protobuf/proto"
"k8s.io/apimachinery/pkg/api/resource"
)
@@ -350,8 +349,13 @@ func NewDeclTypeProvider(rootTypes ...*DeclType) *DeclTypeProvider {
// type-system.
type DeclTypeProvider struct {
registeredTypes map[string]*DeclType
typeProvider ref.TypeProvider
typeAdapter ref.TypeAdapter
typeProvider types.Provider
typeAdapter types.Adapter
recognizeKeywordAsFieldName bool
}
func (rt *DeclTypeProvider) SetRecognizeKeywordAsFieldName(recognize bool) {
rt.recognizeKeywordAsFieldName = recognize
}
func (rt *DeclTypeProvider) EnumValue(enumName string) ref.Val {
@@ -366,7 +370,7 @@ func (rt *DeclTypeProvider) FindIdent(identName string) (ref.Val, bool) {
// as well as a custom ref.TypeProvider.
//
// If the DeclTypeProvider value is nil, an empty []cel.EnvOption set is returned.
func (rt *DeclTypeProvider) EnvOptions(tp ref.TypeProvider) ([]cel.EnvOption, error) {
func (rt *DeclTypeProvider) EnvOptions(tp types.Provider) ([]cel.EnvOption, error) {
if rt == nil {
return []cel.EnvOption{}, nil
}
@@ -382,12 +386,12 @@ func (rt *DeclTypeProvider) EnvOptions(tp ref.TypeProvider) ([]cel.EnvOption, er
// WithTypeProvider returns a new DeclTypeProvider that sets the given TypeProvider
// If the original DeclTypeProvider is nil, the returned DeclTypeProvider is still nil.
func (rt *DeclTypeProvider) WithTypeProvider(tp ref.TypeProvider) (*DeclTypeProvider, error) {
func (rt *DeclTypeProvider) WithTypeProvider(tp types.Provider) (*DeclTypeProvider, error) {
if rt == nil {
return nil, nil
}
var ta ref.TypeAdapter = types.DefaultTypeAdapter
tpa, ok := tp.(ref.TypeAdapter)
var ta types.Adapter = types.DefaultTypeAdapter
tpa, ok := tp.(types.Adapter)
if ok {
ta = tpa
}
@@ -395,41 +399,39 @@ func (rt *DeclTypeProvider) WithTypeProvider(tp ref.TypeProvider) (*DeclTypeProv
typeProvider: tp,
typeAdapter: ta,
registeredTypes: rt.registeredTypes,
recognizeKeywordAsFieldName: rt.recognizeKeywordAsFieldName,
}
for name, declType := range rt.registeredTypes {
tpType, found := tp.FindType(name)
expT, err := declType.ExprType()
if err != nil {
return nil, fmt.Errorf("fail to get cel type: %s", err)
}
if found && !proto.Equal(tpType, expT) {
tpType, found := tp.FindStructType(name)
// cast celType to types.type
expT := declType.CelType()
if found && !expT.IsExactType(tpType) {
return nil, fmt.Errorf(
"type %s definition differs between CEL environment and type provider", name)
}
}
return rtWithTypes, nil
}
// FindType attempts to resolve the typeName provided from the rule's rule-schema, or if not
// FindStructType attempts to resolve the typeName provided from the rule's rule-schema, or if not
// from the embedded ref.TypeProvider.
//
// FindType overrides the default type-finding behavior of the embedded TypeProvider.
// FindStructType overrides the default type-finding behavior of the embedded TypeProvider.
//
// Note, when the type name is based on the Open API Schema, the name will reflect the object path
// where the type definition appears.
func (rt *DeclTypeProvider) FindType(typeName string) (*exprpb.Type, bool) {
func (rt *DeclTypeProvider) FindStructType(typeName string) (*types.Type, bool) {
if rt == nil {
return nil, false
}
declType, found := rt.findDeclType(typeName)
if found {
expT, err := declType.ExprType()
if err != nil {
return expT, false
}
expT := declType.CelType()
return expT, found
}
return rt.typeProvider.FindType(typeName)
return rt.typeProvider.FindStructType(typeName)
}
// FindDeclType returns the CPT type description which can be mapped to a CEL type.
@@ -440,37 +442,41 @@ func (rt *DeclTypeProvider) FindDeclType(typeName string) (*DeclType, bool) {
return rt.findDeclType(typeName)
}
// FindFieldType returns a field type given a type name and field name, if found.
// FindStructFieldNames returns the field names associated with the type, if the type
// is found.
func (rt *DeclTypeProvider) FindStructFieldNames(typeName string) ([]string, bool) {
return []string{}, false
}
// FindStructFieldType returns a field type given a type name and field name, if found.
//
// Note, the type name for an Open API Schema type is likely to be its qualified object path.
// If, in the future an object instance rather than a type name were provided, the field
// resolution might more accurately reflect the expected type model. However, in this case
// concessions were made to align with the existing CEL interfaces.
func (rt *DeclTypeProvider) FindFieldType(typeName, fieldName string) (*ref.FieldType, bool) {
func (rt *DeclTypeProvider) FindStructFieldType(typeName, fieldName string) (*types.FieldType, bool) {
st, found := rt.findDeclType(typeName)
if !found {
return rt.typeProvider.FindFieldType(typeName, fieldName)
return rt.typeProvider.FindStructFieldType(typeName, fieldName)
}
f, found := st.Fields[fieldName]
if rt.recognizeKeywordAsFieldName && !found && celReservedSymbols.Has(fieldName) {
f, found = st.Fields["__"+fieldName+"__"]
}
if found {
ft := f.Type
expT, err := ft.ExprType()
if err != nil {
return nil, false
}
return &ref.FieldType{
expT := ft.CelType()
return &types.FieldType{
Type: expT,
}, true
}
// This could be a dynamic map.
if st.IsMap() {
et := st.ElemType
expT, err := et.ExprType()
if err != nil {
return nil, false
}
return &ref.FieldType{
expT := et.CelType()
return &types.FieldType{
Type: expT,
}, true
}

2
test/integration/apiserver/cel/typeresolution_test.go
View File

@@ -382,7 +382,7 @@ func simpleCompileCEL(schema *spec.Schema, expression string) (cel.Program, erro
}
declType := celopenapi.SchemaDeclType(schema, true).MaybeAssignTypeName("selfType")
rt := commoncel.NewDeclTypeProvider(declType)
opts, err := rt.EnvOptions(env.TypeProvider())
opts, err := rt.EnvOptions(env.CELTypeProvider())
if err != nil {
return nil, err
}