From b03e41404c8893f30b40c3828e09e9a58090da71 Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Fri, 24 Jan 2025 11:25:34 -0500 Subject: [PATCH 1/7] Make schema type definitions recursive. --- pkg/schema/definitions/visitor.go | 27 ++++++++---- pkg/schema/definitions/visitor_test.go | 58 ++++++++++++++++++++------ 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/pkg/schema/definitions/visitor.go b/pkg/schema/definitions/visitor.go index 518bfb07..e8bf8722 100644 --- a/pkg/schema/definitions/visitor.go +++ b/pkg/schema/definitions/visitor.go @@ -20,23 +20,34 @@ func (s *schemaFieldVisitor) VisitArray(array *proto.Array) { // it was kept this way to provide backwards compat with previous endpoints. array.SubType.Accept(s) subField := s.field - field.Type = "array" - field.SubType = subField.Type + field.Type = "array[" + subField.Type + "]" s.field = field } // VisitMap turns a map into a definitionField (stored on the receiver). For maps of complex types, will also visit the // subtype. +//func (s *schemaFieldVisitor) VisitMap(protoMap *proto.Map) { +// field := definitionField{ +// Description: protoMap.GetDescription(), +// } +// // this currently is not recursive and provides little information for nested types- while this isn't optimal, +// // it was kept this way to provide backwards compat with previous endpoints. +// protoMap.SubType.Accept(s) +// subField := s.field +// field.Type = "map[" + subField.Type + "]" +// s.field = field +//} + func (s *schemaFieldVisitor) VisitMap(protoMap *proto.Map) { field := definitionField{ Description: protoMap.GetDescription(), } - // this currently is not recursive and provides little information for nested types- while this isn't optimal, - // it was kept this way to provide backwards compat with previous endpoints. - protoMap.SubType.Accept(s) - subField := s.field - field.Type = "map" - field.SubType = subField.Type + // Recursively visit the value subtype + subVisitor := &schemaFieldVisitor{definitions: s.definitions} + protoMap.SubType.Accept(subVisitor) + subField := subVisitor.field + // Represent the map as "map[string]" + field.Type = "map[string]" + subField.Type s.field = field } diff --git a/pkg/schema/definitions/visitor_test.go b/pkg/schema/definitions/visitor_test.go index c9497d4f..0e1df30c 100644 --- a/pkg/schema/definitions/visitor_test.go +++ b/pkg/schema/definitions/visitor_test.go @@ -72,6 +72,12 @@ var ( Description: "testArbitrary", }, } + protoNestedMap = proto.Map{ + BaseSchema: proto.BaseSchema{ + Description: "nestedMap", + }, + SubType: &protoKind, + } ) func TestSchemaFieldVisitor(t *testing.T) { @@ -87,9 +93,8 @@ func TestSchemaFieldVisitor(t *testing.T) { inputSchema: &protoArray, wantDefinitions: map[string]definition{}, wantField: definitionField{ - Type: "array", + Type: "array[string]", Description: protoArray.Description, - SubType: protoPrimitive.Type, }, }, { @@ -97,9 +102,8 @@ func TestSchemaFieldVisitor(t *testing.T) { inputSchema: &protoMap, wantDefinitions: map[string]definition{}, wantField: definitionField{ - Type: "map", + Type: "map[string]string", Description: protoMap.Description, - SubType: protoPrimitive.Type, }, }, { @@ -136,15 +140,13 @@ func TestSchemaFieldVisitor(t *testing.T) { protoKind.Path.String(): { ResourceFields: map[string]definitionField{ "protoArray": { - Type: "array", + Type: "array[" + protoPrimitive.Type + "]", Description: protoArray.Description, - SubType: protoPrimitive.Type, Required: true, }, "protoMap": { - Type: "map", + Type: "map[" + protoPrimitive.Type + "]string", Description: protoMap.Description, - SubType: protoPrimitive.Type, }, "protoPrimitive": { Type: protoPrimitive.Type, @@ -181,15 +183,13 @@ func TestSchemaFieldVisitor(t *testing.T) { protoKind.Path.String(): { ResourceFields: map[string]definitionField{ "protoArray": { - Type: "array", + Type: "array[string]", Description: protoArray.Description, - SubType: protoPrimitive.Type, Required: true, }, "protoMap": { - Type: "map", + Type: "map[string]string", Description: protoMap.Description, - SubType: protoPrimitive.Type, }, "protoPrimitive": { Type: protoPrimitive.Type, @@ -219,6 +219,40 @@ func TestSchemaFieldVisitor(t *testing.T) { Description: protoArbitrary.Description, }, }, + { + name: "nested map with kind", + inputSchema: &protoNestedMap, + wantDefinitions: map[string]definition{ + protoKind.Path.String(): { + ResourceFields: map[string]definitionField{ + "protoArray": { + Type: "array[string]", + Description: protoArray.Description, + Required: true, + }, + "protoMap": { + Type: "map[string]string", + Description: protoMap.Description, + }, + "protoPrimitive": { + Type: protoPrimitive.Type, + Description: protoPrimitive.Description, + Required: true, + }, + "protoRef": { + Type: protoKind.Path.String(), + Description: protoRef.Description, + }, + }, + Type: protoKind.Path.String(), + Description: protoKind.Description, + }, + }, + wantField: definitionField{ + Type: "map[string]io.cattle.test", + Description: protoNestedMap.Description, + }, + }, } for _, test := range tests { From 34234eaaf3a573af83cd5b90fed123744948bc36 Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Mon, 27 Jan 2025 14:05:14 -0500 Subject: [PATCH 2/7] Update VisitArray --- pkg/schema/definitions/visitor.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/pkg/schema/definitions/visitor.go b/pkg/schema/definitions/visitor.go index e8bf8722..97642f76 100644 --- a/pkg/schema/definitions/visitor.go +++ b/pkg/schema/definitions/visitor.go @@ -16,28 +16,17 @@ func (s *schemaFieldVisitor) VisitArray(array *proto.Array) { field := definitionField{ Description: array.GetDescription(), } - // this currently is not recursive and provides little information for nested types- while this isn't optimal, - // it was kept this way to provide backwards compat with previous endpoints. - array.SubType.Accept(s) - subField := s.field + // Recursively visit the value subtype + subVisitor := &schemaFieldVisitor{definitions: s.definitions} + array.SubType.Accept(subVisitor) + subField := subVisitor.field + // Represent the map as "array[]" field.Type = "array[" + subField.Type + "]" s.field = field } // VisitMap turns a map into a definitionField (stored on the receiver). For maps of complex types, will also visit the // subtype. -//func (s *schemaFieldVisitor) VisitMap(protoMap *proto.Map) { -// field := definitionField{ -// Description: protoMap.GetDescription(), -// } -// // this currently is not recursive and provides little information for nested types- while this isn't optimal, -// // it was kept this way to provide backwards compat with previous endpoints. -// protoMap.SubType.Accept(s) -// subField := s.field -// field.Type = "map[" + subField.Type + "]" -// s.field = field -//} - func (s *schemaFieldVisitor) VisitMap(protoMap *proto.Map) { field := definitionField{ Description: protoMap.GetDescription(), From f4e11e202d9e0694a541542d4bbffeb91eec738d Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Mon, 27 Jan 2025 14:05:38 -0500 Subject: [PATCH 3/7] Fix up handler_test --- pkg/schema/definitions/handler_test.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index 630afc90..5d99d5b3 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/pkg/schema/definitions/handler_test.go @@ -305,13 +305,11 @@ func Test_byID(t *testing.T) { Description: "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata", }, "binaryData": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet.", }, "data": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process.", }, "immutable": { @@ -323,8 +321,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "annotations of the resource", }, "name": { @@ -393,8 +390,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "annotations of the resource", }, "name": { @@ -443,8 +439,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "annotations of the resource", }, "name": { @@ -517,8 +512,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "annotations of the resource", }, "name": { @@ -566,8 +560,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map", - SubType: "string", + Type: "map[string]string", Description: "annotations of the resource", }, "name": { From 879a6d30f30bc70426ce49b2836c8240ea65cd7b Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Tue, 28 Jan 2025 10:23:31 -0500 Subject: [PATCH 4/7] Add test for deeper nesting of maps and arrays --- pkg/schema/definitions/visitor_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/pkg/schema/definitions/visitor_test.go b/pkg/schema/definitions/visitor_test.go index 0e1df30c..4860e746 100644 --- a/pkg/schema/definitions/visitor_test.go +++ b/pkg/schema/definitions/visitor_test.go @@ -253,6 +253,30 @@ func TestSchemaFieldVisitor(t *testing.T) { Description: protoNestedMap.Description, }, }, + { + name: "multi-level nested maps and arrays", + inputSchema: &proto.Map{ + BaseSchema: proto.BaseSchema{ + Description: "multi-level nested structure", + }, + SubType: &proto.Array{ + BaseSchema: proto.BaseSchema{ + Description: "nested array", + }, + SubType: &proto.Map{ + BaseSchema: proto.BaseSchema{ + Description: "deeply nested map", + }, + SubType: &protoPrimitive, + }, + }, + }, + wantDefinitions: map[string]definition{}, + wantField: definitionField{ + Type: "map[string]array[map[string]string]", + Description: "multi-level nested structure", + }, + }, } for _, test := range tests { From b7afe7bdb310d9bafbf38b96da2ca516cefbebd2 Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Tue, 28 Jan 2025 10:54:50 -0500 Subject: [PATCH 5/7] Add test for empty schema --- pkg/schema/definitions/visitor_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/schema/definitions/visitor_test.go b/pkg/schema/definitions/visitor_test.go index 4860e746..3c05460e 100644 --- a/pkg/schema/definitions/visitor_test.go +++ b/pkg/schema/definitions/visitor_test.go @@ -78,6 +78,12 @@ var ( }, SubType: &protoKind, } + protoEmpty = proto.Kind{ + BaseSchema: proto.BaseSchema{ + Description: "emptySchema", + Path: proto.NewPath("io.cattle.empty"), + }, + } ) func TestSchemaFieldVisitor(t *testing.T) { @@ -277,6 +283,21 @@ func TestSchemaFieldVisitor(t *testing.T) { Description: "multi-level nested structure", }, }, + { + name: "empty schema", + inputSchema: &protoEmpty, + wantDefinitions: map[string]definition{ + "io.cattle.empty": { + ResourceFields: map[string]definitionField{}, + Type: "io.cattle.empty", + Description: protoEmpty.Description, + }, + }, + wantField: definitionField{ + Type: "io.cattle.empty", + Description: protoEmpty.Description, + }, + }, } for _, test := range tests { From cd74bc3debb78bc46bfc0bcd0b2b3bfa477f27e1 Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Wed, 29 Jan 2025 07:09:36 -0500 Subject: [PATCH 6/7] Adjust output to match request in issue, update tests to match reality --- pkg/schema/definitions/handler_test.go | 14 +++++++------- pkg/schema/definitions/visitor.go | 7 ++++--- pkg/schema/definitions/visitor_test.go | 12 ++++++------ 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/schema/definitions/handler_test.go b/pkg/schema/definitions/handler_test.go index 5d99d5b3..54507685 100644 --- a/pkg/schema/definitions/handler_test.go +++ b/pkg/schema/definitions/handler_test.go @@ -305,11 +305,11 @@ func Test_byID(t *testing.T) { Description: "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata", }, "binaryData": { - Type: "map[string]string", + Type: "map[string]", Description: "BinaryData contains the binary data. Each key must consist of alphanumeric characters, '-', '_' or '.'. BinaryData can contain byte sequences that are not in the UTF-8 range. The keys stored in BinaryData must not overlap with the ones in the Data field, this is enforced during validation process. Using this field will require 1.10+ apiserver and kubelet.", }, "data": { - Type: "map[string]string", + Type: "map[string]", Description: "Data contains the configuration data. Each key must consist of alphanumeric characters, '-', '_' or '.'. Values with non-UTF-8 byte sequences must use the BinaryData field. The keys stored in Data must not overlap with the keys in the BinaryData field, this is enforced during validation process.", }, "immutable": { @@ -321,7 +321,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map[string]string", + Type: "map[string]", Description: "annotations of the resource", }, "name": { @@ -390,7 +390,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map[string]string", + Type: "map[string]", Description: "annotations of the resource", }, "name": { @@ -439,7 +439,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map[string]string", + Type: "map[string]", Description: "annotations of the resource", }, "name": { @@ -512,7 +512,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map[string]string", + Type: "map[string]", Description: "annotations of the resource", }, "name": { @@ -560,7 +560,7 @@ func Test_byID(t *testing.T) { "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": { ResourceFields: map[string]definitionField{ "annotations": { - Type: "map[string]string", + Type: "map[string]", Description: "annotations of the resource", }, "name": { diff --git a/pkg/schema/definitions/visitor.go b/pkg/schema/definitions/visitor.go index 97642f76..fde60c2e 100644 --- a/pkg/schema/definitions/visitor.go +++ b/pkg/schema/definitions/visitor.go @@ -1,6 +1,7 @@ package definitions import ( + "fmt" "k8s.io/kube-openapi/pkg/util/proto" ) @@ -21,7 +22,7 @@ func (s *schemaFieldVisitor) VisitArray(array *proto.Array) { array.SubType.Accept(subVisitor) subField := subVisitor.field // Represent the map as "array[]" - field.Type = "array[" + subField.Type + "]" + field.Type = fmt.Sprintf("array[%s]", subField.Type) s.field = field } @@ -35,8 +36,8 @@ func (s *schemaFieldVisitor) VisitMap(protoMap *proto.Map) { subVisitor := &schemaFieldVisitor{definitions: s.definitions} protoMap.SubType.Accept(subVisitor) subField := subVisitor.field - // Represent the map as "map[string]" - field.Type = "map[string]" + subField.Type + // Represent the map as "map[]" + field.Type = fmt.Sprintf("map[%s]", subField.Type) s.field = field } diff --git a/pkg/schema/definitions/visitor_test.go b/pkg/schema/definitions/visitor_test.go index 3c05460e..a4bff366 100644 --- a/pkg/schema/definitions/visitor_test.go +++ b/pkg/schema/definitions/visitor_test.go @@ -108,7 +108,7 @@ func TestSchemaFieldVisitor(t *testing.T) { inputSchema: &protoMap, wantDefinitions: map[string]definition{}, wantField: definitionField{ - Type: "map[string]string", + Type: "map[string]", Description: protoMap.Description, }, }, @@ -151,7 +151,7 @@ func TestSchemaFieldVisitor(t *testing.T) { Required: true, }, "protoMap": { - Type: "map[" + protoPrimitive.Type + "]string", + Type: "map[" + protoPrimitive.Type + "]", Description: protoMap.Description, }, "protoPrimitive": { @@ -194,7 +194,7 @@ func TestSchemaFieldVisitor(t *testing.T) { Required: true, }, "protoMap": { - Type: "map[string]string", + Type: "map[string]", Description: protoMap.Description, }, "protoPrimitive": { @@ -237,7 +237,7 @@ func TestSchemaFieldVisitor(t *testing.T) { Required: true, }, "protoMap": { - Type: "map[string]string", + Type: "map[string]", Description: protoMap.Description, }, "protoPrimitive": { @@ -255,7 +255,7 @@ func TestSchemaFieldVisitor(t *testing.T) { }, }, wantField: definitionField{ - Type: "map[string]io.cattle.test", + Type: "map[io.cattle.test]", Description: protoNestedMap.Description, }, }, @@ -279,7 +279,7 @@ func TestSchemaFieldVisitor(t *testing.T) { }, wantDefinitions: map[string]definition{}, wantField: definitionField{ - Type: "map[string]array[map[string]string]", + Type: "map[array[map[string]]]", Description: "multi-level nested structure", }, }, From a6152a497d7ba4bc192b2533c586acf3fb2d8d94 Mon Sep 17 00:00:00 2001 From: Chad Roberts Date: Wed, 29 Jan 2025 09:45:35 -0500 Subject: [PATCH 7/7] fix imports --- pkg/schema/definitions/visitor.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/schema/definitions/visitor.go b/pkg/schema/definitions/visitor.go index fde60c2e..b5272664 100644 --- a/pkg/schema/definitions/visitor.go +++ b/pkg/schema/definitions/visitor.go @@ -2,6 +2,7 @@ package definitions import ( "fmt" + "k8s.io/kube-openapi/pkg/util/proto" )