Skip to content

Commit

Permalink
Fix nil pointer dereference in selectable fields check When checking …
Browse files Browse the repository at this point in the history
…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>

Kubernetes-commit: 4fd08097171398a5f1d69a9ec19db2773c1d49d4
  • Loading branch information
jyh071116 authored and k8s-publishing-bot committed Nov 11, 2024
1 parent 6ca85ae commit 58ad8a0
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/controller/openapi/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
25 changes: 23 additions & 2 deletions pkg/controller/openapi/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
`
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 58ad8a0

Please sign in to comment.