From de4e04baf6e2e4b8e70cb01f1bb27e2189f4a0b4 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 5 Nov 2025 15:08:15 -0500 Subject: [PATCH] Fix NPE in CEl accessors of additionalProperties=true objects --- .../policy/validating/typechecking_test.go | 53 +++++++++++++++++++ .../apiserver/pkg/cel/openapi/adaptor.go | 3 ++ .../apiserver/pkg/cel/openapi/schemas_test.go | 27 ++++++++++ 3 files changed, 83 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking_test.go index 4da14b92912..2a78ac62847 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/typechecking_test.go @@ -297,6 +297,26 @@ func TestTypeCheck(t *testing.T) { }, }}, }} + + reproducerPolicy := &v1.ValidatingAdmissionPolicy{Spec: v1.ValidatingAdmissionPolicySpec{ + Validations: []v1.Validation{ + { + Expression: "has(object.spec.text)", + }, + }, + MatchConstraints: &v1.MatchResources{ResourceRules: []v1.NamedRuleWithOperations{ + { + RuleWithOperations: v1.RuleWithOperations{ + Rule: v1.Rule{ + APIGroups: []string{"example.com"}, + APIVersions: []string{"v1alpha1"}, + Resources: []string{"reproducers"}, + }, + }, + }, + }}, + }} + for _, tc := range []struct { name string schemaToReturn *spec.Schema @@ -437,6 +457,39 @@ func TestTypeCheck(t *testing.T) { toContain("found no matching overload for 'allowed' applied to 'kubernetes.authorization.Authorizer"), }, }, + { + name: "additionalProperties: true", + policy: reproducerPolicy, + schemaToReturn: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "spec": { + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "text": *spec.StringProperty(), + }, + }, + }, + "status": { + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "problematicProperty": { + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{Allows: true}, + }, + }, + }, + }, + }, + }, + }, + }, + assertions: []assertionFunc{toBeEmpty}, + }, { name: "variables valid", policy: &v1.ValidatingAdmissionPolicy{Spec: v1.ValidatingAdmissionPolicySpec{ diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/adaptor.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/adaptor.go index bc7b0d8c959..f95ec2a3722 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/adaptor.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/adaptor.go @@ -36,6 +36,9 @@ type SchemaOrBool struct { } func (sb *SchemaOrBool) Schema() common.Schema { + if sb.SchemaOrBool.Schema == nil { + return nil + } return &Schema{Schema: sb.SchemaOrBool.Schema} } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/openapi/schemas_test.go b/staging/src/k8s.io/apiserver/pkg/cel/openapi/schemas_test.go index c48aea73867..9951470c5c1 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/openapi/schemas_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/openapi/schemas_test.go @@ -29,6 +29,33 @@ import ( ) func TestSchemaDeclType(t *testing.T) { + t.Run("a schema with additionalProperties: true should be an valid object type", func(t *testing.T) { + schema := &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "foo": { + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{Allows: true}, + }, + }, + }, + }, + } + declType := SchemaDeclType(schema, false) + if declType == nil { + t.Fatal("SchemaDeclType returned nil") + } + fooField, found := declType.FindField("foo") + if !found { + t.Fatal("field 'foo' not found") + } + fooType := fooField.Type + if fooType.TypeName() != "object" { + t.Fatalf("expected foo to be a object, but got %s", fooType.TypeName()) + } + }) ts := testSchema() cust := SchemaDeclType(ts, false) if cust.TypeName() != "object" {