Skip to content

Commit 834658c

Browse files
authored
Fix kubectl explain bug when additionalProperties in schema defines as boolean. (kubernetes#124506)
* Fix kubectl explain bug when additionalProperties in schema defines as: `additionalProperties: true` to ignore iterating. * trigger error on kubectl explain with integration test on crd with non bool additionalfields * add changes to fix the problem * replace sleep with loop and retry for kubectl explain integration test * replaced testdata file with inline create
1 parent 44c4548 commit 834658c

File tree

4 files changed

+98
-4
lines changed

4 files changed

+98
-4
lines changed

staging/src/k8s.io/kubectl/pkg/explain/v2/templates/plaintext.tmpl

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Takes dictionary as argument with keys:
118118
{{- else if $resolved.items -}}
119119
{{- /* If the schema is an array then traverse the array item fields */ -}}
120120
{{- template "output" (set $nextContext "schema" $resolved.items) -}}
121-
{{- else if $resolved.additionalProperties -}}
121+
{{- else if and $resolved.additionalProperties (not (eq "bool" (printf "%T" $resolved.additionalProperties))) -}}
122122
{{- /* If the schema is a map then traverse the map item fields */ -}}
123123
{{- template "output" (set $nextContext "schema" $resolved.additionalProperties) -}}
124124
{{- else -}}
@@ -182,7 +182,9 @@ Takes dictionary as argument with keys:
182182
{{- template "fieldList" (set $nextContext "schema" .)}}
183183
{{- end -}}
184184
{{- if $resolved.items}}{{- template "fieldList" (set $nextContext "schema" $resolved.items)}}{{end}}
185-
{{- if $resolved.additionalProperties}}{{- template "fieldList" (set $nextContext "schema" $resolved.additionalProperties)}}{{end}}
185+
{{- if and $resolved.additionalProperties (not (eq "bool" (printf "%T" $resolved.additionalProperties))) -}}
186+
{{- template "fieldList" (set $nextContext "schema" $resolved.additionalProperties)}}
187+
{{- end -}}
186188
{{- end -}}
187189
{{- end -}}
188190

@@ -235,7 +237,7 @@ Takes dictionary as argument with keys:
235237
{{- if .items -}}
236238
{{- template "description" (set $ "schema" .items) -}}
237239
{{- end -}}
238-
{{- if .additionalProperties -}}
240+
{{- if and .additionalProperties (not (eq "bool" (printf "%T" .additionalProperties))) -}}
239241
{{- template "description" (set $ "schema" .additionalProperties) -}}
240242
{{- end -}}
241243
{{- end -}}
@@ -256,7 +258,7 @@ Takes dictionary as argument with keys:
256258
{{- with $.schema -}}
257259
{{- if .items -}}
258260
[]{{template "typeGuess" (set $ "schema" .items)}}
259-
{{- else if .additionalProperties -}}
261+
{{- else if and .additionalProperties (not (eq "bool" (printf "%T" .additionalProperties))) -}}
260262
map[string]{{template "typeGuess" (set $ "schema" .additionalProperties)}}
261263
{{- else if and .allOf (not .properties) (eq (len .allOf) 1) -}}
262264
{{- /* If allOf has a single element and there are no direct

staging/src/k8s.io/kubectl/pkg/explain/v2/templates/plaintext_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,20 @@ func TestPlaintext(t *testing.T) {
392392
checkEquals("string"),
393393
},
394394
},
395+
{
396+
// Shows that the typeguess template works with boolean additionalProperties
397+
Name: "True additionalProperties",
398+
Subtemplate: "typeGuess",
399+
Context: map[string]any{
400+
"schema": map[string]any{
401+
"type": "string",
402+
"additionalProperties": true,
403+
},
404+
},
405+
Checks: []check{
406+
checkEquals("string"),
407+
},
408+
},
395409
{
396410
// Show that a ref to a primitive type uses the referred type's type
397411
Name: "PrimitiveRef",

test/cmd/discovery.sh

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,73 @@ __EOF__
407407
set +o nounset
408408
set +o errexit
409409
}
410+
411+
run_explain_crd_with_additional_properties_tests() {
412+
set -o nounset
413+
set -o errexit
414+
415+
create_and_use_new_namespace
416+
kube::log::status "Testing explain with custom CRD that uses additionalProperties as non boolean field"
417+
418+
kubectl create -f - << __EOF__
419+
apiVersion: apiextensions.k8s.io/v1
420+
kind: CustomResourceDefinition
421+
metadata:
422+
name: mock-resources.test.com
423+
spec:
424+
group: test.com
425+
scope: Namespaced
426+
names:
427+
plural: mock-resources
428+
singular: mock-resource
429+
kind: MockResource
430+
listKind: MockResources
431+
versions:
432+
- name: v1
433+
storage: true
434+
served: true
435+
schema:
436+
openAPIV3Schema:
437+
type: object
438+
properties:
439+
spec:
440+
type: object
441+
properties:
442+
test:
443+
type: object
444+
additionalProperties:
445+
type: array
446+
items:
447+
type: string
448+
additionalProperties: true
449+
__EOF__
450+
451+
kube::test::wait_object_assert customresourcedefinitions "{{range.items}}{{if eq ${id_field:?} \"mock-resources.test.com\"}}{{$id_field}}:{{end}}{{end}}" 'mock-resources.test.com:'
452+
453+
retries=0
454+
max_retries=5
455+
456+
# First try would get non zero exit code so we disable immediate exit here
457+
set +o errexit
458+
459+
# Loop until `kubectl explain` returns a zero exit code or maximum retries reached
460+
until output_message=$(kubectl explain mock-resource) > /dev/null || [ $retries -eq $max_retries ]; do
461+
kube::log::status "Retrying kubectl explain..."
462+
((retries++))
463+
sleep 1
464+
done
465+
466+
set -o errexit
467+
468+
output_message=$(kubectl explain mock-resource)
469+
kube::test::if_has_string "${output_message}" 'FIELDS:'
470+
471+
output_message=$(kubectl explain mock-resource --recursive)
472+
kube::test::if_has_string "${output_message}" 'FIELDS:'
473+
474+
# Cleanup
475+
kubectl delete crd mock-resources.test.com
476+
477+
set +o nounset
478+
set +o errexit
479+
}

test/cmd/legacy-script.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,14 @@ runTests() {
526526
record_command run_ambiguous_shortname_tests
527527
fi
528528

529+
################
530+
# Explain crd #
531+
################
532+
533+
if kube::test::if_supports_resource "${customresourcedefinitions}" ; then
534+
record_command run_explain_crd_with_additional_properties_tests
535+
fi
536+
529537
#########################
530538
# Assert categories #
531539
#########################

0 commit comments

Comments
 (0)