Fix for the 'kubectl explain crd --recursive' goes into an infinite l…

…oop issue
kubernetes · Oct 30, 2018 · 05a461c · 05a461c
1 parent 03a145d
commit 05a461c
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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	<Object>
+   referencefield	<Object>
+   referencesarray	<[]Object>
+field2	<Object>
+   reference	<Object>
+      referencefield	<Object>
+      referencesarray	<[]Object>
+   string	<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
@@ -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"
+        }
+      }
+    }
+  }
+}