Skip to content

Commit d7de3e5

Browse files
authored
Merge pull request kubernetes#70428 from vithati/users/vithati/kubectlissue546
Fix for the 'kubectl explain crd --recursive' goes into an infinite loop issue
2 parents 26083c3 + 05a461c commit d7de3e5

File tree

4 files changed

+113
-1
lines changed

4 files changed

+113
-1
lines changed

pkg/kubectl/explain/BUILD

+4-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ go_test(
4646
"recursive_fields_printer_test.go",
4747
"typename_test.go",
4848
],
49-
data = ["test-swagger.json"],
49+
data = [
50+
"test-recursive-swagger.json",
51+
"test-swagger.json",
52+
],
5053
embed = [":go_default_library"],
5154
deps = [
5255
"//pkg/kubectl/cmd/util/openapi/testing:go_default_library",

pkg/kubectl/explain/recursive_fields_printer.go

+6
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type recursiveFieldsPrinter struct {
3030

3131
var _ proto.SchemaVisitor = &recursiveFieldsPrinter{}
3232
var _ fieldsPrinter = &recursiveFieldsPrinter{}
33+
var visitedReferences = map[string]struct{}{}
3334

3435
// VisitArray is just a passthrough.
3536
func (f *recursiveFieldsPrinter) VisitArray(a *proto.Array) {
@@ -64,7 +65,12 @@ func (f *recursiveFieldsPrinter) VisitPrimitive(p *proto.Primitive) {
6465

6566
// VisitReference is just a passthrough.
6667
func (f *recursiveFieldsPrinter) VisitReference(r proto.Reference) {
68+
if _, ok := visitedReferences[r.Reference()]; ok {
69+
return
70+
}
71+
visitedReferences[r.Reference()] = struct{}{}
6772
r.SubSchema().Accept(f)
73+
delete(visitedReferences, r.Reference())
6874
}
6975

7076
// PrintFields will recursively print all the fields for the given

pkg/kubectl/explain/recursive_fields_printer_test.go

+40
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"testing"
2222

2323
"k8s.io/apimachinery/pkg/runtime/schema"
24+
tst "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/testing"
2425
)
2526

2627
func TestRecursiveFields(t *testing.T) {
@@ -59,3 +60,42 @@ field2 <[]map[string]string>
5960
t.Errorf("Got:\n%v\nWant:\n%v\n", buf.String(), want)
6061
}
6162
}
63+
64+
func TestRecursiveFieldsWithSelfReferenceObjects(t *testing.T) {
65+
var resources = tst.NewFakeResources("test-recursive-swagger.json")
66+
schema := resources.LookupResource(schema.GroupVersionKind{
67+
Group: "",
68+
Version: "v2",
69+
Kind: "OneKind",
70+
})
71+
if schema == nil {
72+
t.Fatal("Couldn't find schema v2.OneKind")
73+
}
74+
75+
want := `field1 <Object>
76+
referencefield <Object>
77+
referencesarray <[]Object>
78+
field2 <Object>
79+
reference <Object>
80+
referencefield <Object>
81+
referencesarray <[]Object>
82+
string <string>
83+
`
84+
85+
buf := bytes.Buffer{}
86+
f := Formatter{
87+
Writer: &buf,
88+
Wrap: 80,
89+
}
90+
s, err := LookupSchemaForField(schema, []string{})
91+
if err != nil {
92+
t.Fatalf("Invalid path %v: %v", []string{}, err)
93+
}
94+
if err := (fieldsPrinterBuilder{Recursive: true}).BuildFieldsPrinter(&f).PrintFields(s); err != nil {
95+
t.Fatalf("Failed to print fields: %v", err)
96+
}
97+
got := buf.String()
98+
if got != want {
99+
t.Errorf("Got:\n%v\nWant:\n%v\n", buf.String(), want)
100+
}
101+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
{
2+
"swagger": "2.0",
3+
"info": {
4+
"title": "Kubernetes",
5+
"version": "v1.9.0"
6+
},
7+
"paths": {},
8+
"definitions": {
9+
"OneKind": {
10+
"description": "OneKind has a short description",
11+
"required": [
12+
"field1"
13+
],
14+
"properties": {
15+
"field1": {
16+
"description": "This is first reference field",
17+
"$ref": "#/definitions/ReferenceKind"
18+
},
19+
"field2": {
20+
"description": "This is other kind field with string and reference",
21+
"$ref": "#/definitions/OtherKind"
22+
}
23+
},
24+
"x-kubernetes-group-version-kind": [
25+
{
26+
"group": "",
27+
"kind": "OneKind",
28+
"version": "v2"
29+
}
30+
]
31+
},
32+
"ReferenceKind": {
33+
"description": "This is reference Kind",
34+
"properties": {
35+
"referencefield": {
36+
"description": "This is reference to itself.",
37+
"$ref": "#/definitions/ReferenceKind"
38+
},
39+
"referencesarray": {
40+
"description": "This is an array of references",
41+
"type": "array",
42+
"items": {
43+
"description": "This is reference object",
44+
"$ref": "#/definitions/ReferenceKind"
45+
}
46+
}
47+
}
48+
},
49+
"OtherKind": {
50+
"description": "This is other kind with string and reference fields",
51+
"properties": {
52+
"string": {
53+
"description": "This string must be a string",
54+
"type": "string"
55+
},
56+
"reference": {
57+
"description": "This is reference field.",
58+
"$ref": "#/definitions/ReferenceKind"
59+
}
60+
}
61+
}
62+
}
63+
}

0 commit comments

Comments
 (0)