From 05a461c17bc54d064466b7aff3b7318df1fe2a72 Mon Sep 17 00:00:00 2001 From: Vikranth Thati Date: Wed, 31 Oct 2018 03:30:46 +0530 Subject: [PATCH] Fix for the 'kubectl explain crd --recursive' goes into an infinite loop issue --- pkg/kubectl/explain/BUILD | 5 +- .../explain/recursive_fields_printer.go | 6 ++ .../explain/recursive_fields_printer_test.go | 40 ++++++++++++ .../explain/test-recursive-swagger.json | 63 +++++++++++++++++++ 4 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 pkg/kubectl/explain/test-recursive-swagger.json diff --git a/pkg/kubectl/explain/BUILD b/pkg/kubectl/explain/BUILD index 14815dad96a6..b70f8476cf40 100644 --- a/pkg/kubectl/explain/BUILD +++ b/pkg/kubectl/explain/BUILD @@ -46,7 +46,10 @@ go_test( "recursive_fields_printer_test.go", "typename_test.go", ], - data = ["test-swagger.json"], + data = [ + "test-recursive-swagger.json", + "test-swagger.json", + ], embed = [":go_default_library"], deps = [ "//pkg/kubectl/cmd/util/openapi/testing:go_default_library", diff --git a/pkg/kubectl/explain/recursive_fields_printer.go b/pkg/kubectl/explain/recursive_fields_printer.go index 95638c85adfe..6429a73264eb 100644 --- a/pkg/kubectl/explain/recursive_fields_printer.go +++ b/pkg/kubectl/explain/recursive_fields_printer.go @@ -30,6 +30,7 @@ type recursiveFieldsPrinter struct { var _ proto.SchemaVisitor = &recursiveFieldsPrinter{} var _ fieldsPrinter = &recursiveFieldsPrinter{} +var visitedReferences = map[string]struct{}{} // VisitArray is just a passthrough. func (f *recursiveFieldsPrinter) VisitArray(a *proto.Array) { @@ -64,7 +65,12 @@ func (f *recursiveFieldsPrinter) VisitPrimitive(p *proto.Primitive) { // VisitReference is just a passthrough. func (f *recursiveFieldsPrinter) VisitReference(r proto.Reference) { + if _, ok := visitedReferences[r.Reference()]; ok { + return + } + visitedReferences[r.Reference()] = struct{}{} r.SubSchema().Accept(f) + delete(visitedReferences, r.Reference()) } // PrintFields will recursively print all the fields for the given diff --git a/pkg/kubectl/explain/recursive_fields_printer_test.go b/pkg/kubectl/explain/recursive_fields_printer_test.go index 75571ab4ee5b..9a9366881744 100644 --- a/pkg/kubectl/explain/recursive_fields_printer_test.go +++ b/pkg/kubectl/explain/recursive_fields_printer_test.go @@ -21,6 +21,7 @@ import ( "testing" "k8s.io/apimachinery/pkg/runtime/schema" + tst "k8s.io/kubernetes/pkg/kubectl/cmd/util/openapi/testing" ) func TestRecursiveFields(t *testing.T) { @@ -59,3 +60,42 @@ field2 <[]map[string]string> t.Errorf("Got:\n%v\nWant:\n%v\n", buf.String(), want) } } + +func TestRecursiveFieldsWithSelfReferenceObjects(t *testing.T) { + var resources = tst.NewFakeResources("test-recursive-swagger.json") + schema := resources.LookupResource(schema.GroupVersionKind{ + Group: "", + Version: "v2", + Kind: "OneKind", + }) + if schema == nil { + t.Fatal("Couldn't find schema v2.OneKind") + } + + want := `field1 + referencefield + referencesarray <[]Object> +field2 + reference + referencefield + referencesarray <[]Object> + string +` + + buf := bytes.Buffer{} + f := Formatter{ + Writer: &buf, + Wrap: 80, + } + s, err := LookupSchemaForField(schema, []string{}) + if err != nil { + t.Fatalf("Invalid path %v: %v", []string{}, err) + } + if err := (fieldsPrinterBuilder{Recursive: true}).BuildFieldsPrinter(&f).PrintFields(s); err != nil { + t.Fatalf("Failed to print fields: %v", err) + } + got := buf.String() + if got != want { + t.Errorf("Got:\n%v\nWant:\n%v\n", buf.String(), want) + } +} diff --git a/pkg/kubectl/explain/test-recursive-swagger.json b/pkg/kubectl/explain/test-recursive-swagger.json new file mode 100644 index 000000000000..1ae79855d393 --- /dev/null +++ b/pkg/kubectl/explain/test-recursive-swagger.json @@ -0,0 +1,63 @@ +{ + "swagger": "2.0", + "info": { + "title": "Kubernetes", + "version": "v1.9.0" + }, + "paths": {}, + "definitions": { + "OneKind": { + "description": "OneKind has a short description", + "required": [ + "field1" + ], + "properties": { + "field1": { + "description": "This is first reference field", + "$ref": "#/definitions/ReferenceKind" + }, + "field2": { + "description": "This is other kind field with string and reference", + "$ref": "#/definitions/OtherKind" + } + }, + "x-kubernetes-group-version-kind": [ + { + "group": "", + "kind": "OneKind", + "version": "v2" + } + ] + }, + "ReferenceKind": { + "description": "This is reference Kind", + "properties": { + "referencefield": { + "description": "This is reference to itself.", + "$ref": "#/definitions/ReferenceKind" + }, + "referencesarray": { + "description": "This is an array of references", + "type": "array", + "items": { + "description": "This is reference object", + "$ref": "#/definitions/ReferenceKind" + } + } + } + }, + "OtherKind": { + "description": "This is other kind with string and reference fields", + "properties": { + "string": { + "description": "This string must be a string", + "type": "string" + }, + "reference": { + "description": "This is reference field.", + "$ref": "#/definitions/ReferenceKind" + } + } + } + } +}