From 4fd08097171398a5f1d69a9ec19db2773c1d49d4 Mon Sep 17 00:00:00 2001
From: jyh071116 <jyh071116@gmail.com>
Date: Mon, 11 Nov 2024 17:03:08 +0900
Subject: [PATCH] Fix nil pointer dereference in selectable fields check When
 checking specVersion.SelectableFields, if specVersion is nil, a nil pointer
 dereference could occur. This change updates the conditional to use ||
 instead of &&, ensuring that the check for specVersion being nil happens
 first, avoiding potential runtime panics.

Additionally, a new test case has been added to validate this condition, ensuring safe handling of nil values for specVersion and empty SelectableFields.
Signed-off-by: pirates <whalehunting.bob@gmail.com>
---
 .../pkg/controller/openapi/builder/builder.go |  2 +-
 .../openapi/builder/builder_test.go           | 25 +++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go
index 45e4604d3e0..51cde0cac15 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go
@@ -623,7 +623,7 @@ func buildSelectableFields(crd *apiextensionsv1.CustomResourceDefinition, versio
 			break
 		}
 	}
-	if specVersion == nil && len(specVersion.SelectableFields) == 0 {
+	if specVersion == nil || len(specVersion.SelectableFields) == 0 {
 		return nil
 	}
 	selectableFields := make([]any, len(specVersion.SelectableFields))
diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go
index 0283bcc4b82..1a30e2b8230 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder_test.go
@@ -45,31 +45,41 @@ func TestNewBuilder(t *testing.T) {
 		wantedSchema      string
 		wantedItemsSchema string
 
-		v2 bool // produce OpenAPIv2
+		v2                bool // produce OpenAPIv2
+		includeSelectable bool // include selectable fields
+		version           string
 	}{
 		{
 			"nil",
 			"",
 			`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`, `{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
+			false,
+			"v1",
 		},
 		{"with properties",
 			`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
 			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
+			false,
+			"v1",
 		},
 		{"type only",
 			`{"type":"object"}`,
 			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
+			false,
+			"v1",
 		},
 		{"preserve unknown at root v2",
 			`{"type":"object","x-kubernetes-preserve-unknown-fields":true}`,
 			`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
+			false,
+			"v1",
 		},
 		{"with extensions",
 			`
@@ -173,6 +183,17 @@ func TestNewBuilder(t *testing.T) {
 }`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
 			true,
+			false,
+			"v1",
+		},
+		{
+			"include selectable fields with different version",
+			`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
+			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v2"}]}`,
+			`{"$ref":"#/definitions/io.k8s.bar.v2.Foo"}`,
+			true,
+			true,
+			"v2",
 		},
 	}
 	for _, tt := range tests {
@@ -212,7 +233,7 @@ func TestNewBuilder(t *testing.T) {
 					},
 					Scope: apiextensionsv1.NamespaceScoped,
 				},
-			}, "v1", schema, Options{V2: tt.v2})
+			}, tt.version, schema, Options{V2: tt.v2, IncludeSelectableFields: tt.includeSelectable})
 
 			var wantedSchema, wantedItemsSchema spec.Schema
 			if err := json.Unmarshal([]byte(tt.wantedSchema), &wantedSchema); err != nil {