From cd19e59c701069f0243a837cdf5fd70115644715 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Wed, 30 Jul 2025 19:27:51 +0200 Subject: [PATCH 1/6] map metadata to objectmeta On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/exports_test.go | 8 ++ gateway/schema/metadata_inference_test.go | 92 +++++++++++++++++++++++ gateway/schema/schema.go | 55 +++++++++++++- 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 gateway/schema/metadata_inference_test.go diff --git a/gateway/schema/exports_test.go b/gateway/schema/exports_test.go index 40c2aab4..2522b934 100644 --- a/gateway/schema/exports_test.go +++ b/gateway/schema/exports_test.go @@ -21,3 +21,11 @@ func (g *Gateway) GenerateTypeNameForTest(typePrefix string, fieldPath []string) func SanitizeFieldNameForTest(name string) string { return sanitizeFieldName(name) } + +func (g *Gateway) ShouldInferAsObjectMetaForTest(fieldPath []string) bool { + return g.shouldInferAsObjectMeta(fieldPath) +} + +func (g *Gateway) GetObjectMetaTypeForTest() (interface{}, interface{}, error) { + return g.getObjectMetaType() +} diff --git a/gateway/schema/metadata_inference_test.go b/gateway/schema/metadata_inference_test.go new file mode 100644 index 00000000..37a6a001 --- /dev/null +++ b/gateway/schema/metadata_inference_test.go @@ -0,0 +1,92 @@ +package schema_test + +import ( + "testing" + + "github.com/openmfp/kubernetes-graphql-gateway/gateway/schema" +) + +func TestShouldInferAsObjectMeta(t *testing.T) { + g := schema.GetGatewayForTest(map[string]string{}) + + tests := []struct { + name string + fieldPath []string + expected bool + }{ + { + name: "marketplace_entry_api_export_metadata", + fieldPath: []string{"spec", "apiExport", "metadata"}, + expected: true, + }, + { + name: "marketplace_entry_provider_metadata_metadata", + fieldPath: []string{"spec", "providerMetadata", "metadata"}, + expected: true, + }, + { + name: "root_metadata_should_not_infer", + fieldPath: []string{"metadata"}, + expected: false, + }, + { + name: "non_metadata_field", + fieldPath: []string{"spec", "containers"}, + expected: false, + }, + { + name: "metadata_but_wrong_path", + fieldPath: []string{"spec", "someOtherField", "metadata"}, + expected: false, + }, + { + name: "empty_field_path", + fieldPath: []string{}, + expected: false, + }, + { + name: "metadata_at_wrong_level", + fieldPath: []string{"data", "metadata"}, + expected: false, + }, + { + name: "deeply_nested_but_not_whitelisted", + fieldPath: []string{"spec", "template", "spec", "metadata"}, + expected: false, + }, + { + name: "partial_match_should_not_infer", + fieldPath: []string{"spec", "apiExport"}, + expected: false, + }, + { + name: "case_sensitive_metadata", + fieldPath: []string{"spec", "apiExport", "Metadata"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := g.ShouldInferAsObjectMetaForTest(tt.fieldPath) + if got != tt.expected { + t.Errorf("ShouldInferAsObjectMetaForTest(%v) = %v, want %v", tt.fieldPath, got, tt.expected) + } + }) + } +} + +func TestGetObjectMetaType_Fallback(t *testing.T) { + // Test that getObjectMetaType doesn't panic and returns something + g := schema.GetGatewayForTest(map[string]string{}) + + outputType, inputType, err := g.GetObjectMetaTypeForTest() + + if err != nil { + t.Errorf("GetObjectMetaTypeForTest() unexpected error: %v", err) + } + + if outputType == nil || inputType == nil { + t.Errorf("GetObjectMetaTypeForTest() should return non-nil types as fallback, got outputType=%v, inputType=%v", outputType, inputType) + } +} diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index 96e18c3b..e803e3d8 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -453,8 +453,59 @@ func (g *Gateway) handleObjectFieldSpecType(fieldSpec spec.Schema, typePrefix st } } - // It's an empty object - return graphql.String, graphql.String, nil + // Check if this should be ObjectMeta + if g.shouldInferAsObjectMeta(fieldPath) { + return g.getObjectMetaType() + } + + // It's an empty object, use StringMap as fallback + return stringMapScalar, stringMapScalar, nil +} + +// shouldInferAsObjectMeta checks if a field path should be inferred as ObjectMeta +func (g *Gateway) shouldInferAsObjectMeta(fieldPath []string) bool { + if len(fieldPath) == 0 { + return false + } + + // Check if the last field in the path is "metadata" + lastField := fieldPath[len(fieldPath)-1] + if lastField != "metadata" { + return false + } + + // Define whitelist of known metadata fields that should be ObjectMeta + pathStr := strings.Join(fieldPath, ".") + knownMetadataPaths := []string{ + "spec.apiExport.metadata", + "spec.providerMetadata.metadata", + } + + for _, knownPath := range knownMetadataPaths { + if pathStr == knownPath { + return true + } + } + + return false +} + +// getObjectMetaType returns the ObjectMeta type from the schema definitions +func (g *Gateway) getObjectMetaType() (graphql.Output, graphql.Input, error) { + objectMetaKey := "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta" + + if g.definitions != nil { + if objectMetaSchema, exists := g.definitions[objectMetaKey]; exists { + return g.handleObjectFieldSpecType(objectMetaSchema, "ObjectMeta", []string{}, make(map[string]bool)) + } + } + + // Log warning when ObjectMeta definition is missing but expected + if g.log != nil { + g.log.Error().Msg("ObjectMeta definition not found in schema, falling back to StringMap") + } + + return stringMapScalar, stringMapScalar, nil } func (g *Gateway) generateTypeName(typePrefix string, fieldPath []string) string { From ebd387166c426b0e43143387802f48c8ab9e1814 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Wed, 30 Jul 2025 19:31:59 +0200 Subject: [PATCH 2/6] language On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/metadata_inference_test.go | 2 +- gateway/schema/schema.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gateway/schema/metadata_inference_test.go b/gateway/schema/metadata_inference_test.go index 37a6a001..8d6fbdba 100644 --- a/gateway/schema/metadata_inference_test.go +++ b/gateway/schema/metadata_inference_test.go @@ -50,7 +50,7 @@ func TestShouldInferAsObjectMeta(t *testing.T) { expected: false, }, { - name: "deeply_nested_but_not_whitelisted", + name: "deeply_nested_but_not_allowlisted", fieldPath: []string{"spec", "template", "spec", "metadata"}, expected: false, }, diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index e803e3d8..724e8ada 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -474,7 +474,7 @@ func (g *Gateway) shouldInferAsObjectMeta(fieldPath []string) bool { return false } - // Define whitelist of known metadata fields that should be ObjectMeta + // Define allowlist of known metadata fields that should be ObjectMeta pathStr := strings.Join(fieldPath, ".") knownMetadataPaths := []string{ "spec.apiExport.metadata", From 1d69f317fce606467f6a5c7729daabef717301cb Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Wed, 30 Jul 2025 19:38:06 +0200 Subject: [PATCH 3/6] use existing field name instead of creating a new one On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/schema.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index 724e8ada..fb1ed4f7 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -494,9 +494,16 @@ func (g *Gateway) shouldInferAsObjectMeta(fieldPath []string) bool { func (g *Gateway) getObjectMetaType() (graphql.Output, graphql.Input, error) { objectMetaKey := "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta" + // First check if the ObjectMeta type is already cached (most common case) + if existingType, exists := g.typesCache[objectMetaKey]; exists { + existingInputType := g.inputTypesCache[objectMetaKey] + return existingType, existingInputType, nil + } + + // If not cached, try to generate it using the same key as normal $ref processing if g.definitions != nil { if objectMetaSchema, exists := g.definitions[objectMetaKey]; exists { - return g.handleObjectFieldSpecType(objectMetaSchema, "ObjectMeta", []string{}, make(map[string]bool)) + return g.handleObjectFieldSpecType(objectMetaSchema, objectMetaKey, []string{}, make(map[string]bool)) } } From 9b7a8ed135db8c47a647f24f302f34d863ce0278 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 31 Jul 2025 09:55:29 +0200 Subject: [PATCH 4/6] removed allowlist On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/metadata_inference_test.go | 37 ++++++----------------- gateway/schema/schema.go | 20 +----------- 2 files changed, 10 insertions(+), 47 deletions(-) diff --git a/gateway/schema/metadata_inference_test.go b/gateway/schema/metadata_inference_test.go index 8d6fbdba..0450d507 100644 --- a/gateway/schema/metadata_inference_test.go +++ b/gateway/schema/metadata_inference_test.go @@ -15,53 +15,34 @@ func TestShouldInferAsObjectMeta(t *testing.T) { expected bool }{ { - name: "marketplace_entry_api_export_metadata", - fieldPath: []string{"spec", "apiExport", "metadata"}, + name: "one_level_deep_metadata", + fieldPath: []string{"metadata"}, expected: true, }, { - name: "marketplace_entry_provider_metadata_metadata", - fieldPath: []string{"spec", "providerMetadata", "metadata"}, + name: "two_level_deep_metadata", + fieldPath: []string{"spec", "metadata"}, expected: true, }, { - name: "root_metadata_should_not_infer", - fieldPath: []string{"metadata"}, - expected: false, + name: "three_level_deep_metadata", + fieldPath: []string{"spec", "apiExport", "metadata"}, + expected: true, }, + { name: "non_metadata_field", fieldPath: []string{"spec", "containers"}, expected: false, }, - { - name: "metadata_but_wrong_path", - fieldPath: []string{"spec", "someOtherField", "metadata"}, - expected: false, - }, { name: "empty_field_path", fieldPath: []string{}, expected: false, }, - { - name: "metadata_at_wrong_level", - fieldPath: []string{"data", "metadata"}, - expected: false, - }, - { - name: "deeply_nested_but_not_allowlisted", - fieldPath: []string{"spec", "template", "spec", "metadata"}, - expected: false, - }, - { - name: "partial_match_should_not_infer", - fieldPath: []string{"spec", "apiExport"}, - expected: false, - }, { name: "case_sensitive_metadata", - fieldPath: []string{"spec", "apiExport", "Metadata"}, + fieldPath: []string{"Metadata"}, expected: false, }, } diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index fb1ed4f7..46ada63d 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -469,25 +469,7 @@ func (g *Gateway) shouldInferAsObjectMeta(fieldPath []string) bool { } // Check if the last field in the path is "metadata" - lastField := fieldPath[len(fieldPath)-1] - if lastField != "metadata" { - return false - } - - // Define allowlist of known metadata fields that should be ObjectMeta - pathStr := strings.Join(fieldPath, ".") - knownMetadataPaths := []string{ - "spec.apiExport.metadata", - "spec.providerMetadata.metadata", - } - - for _, knownPath := range knownMetadataPaths { - if pathStr == knownPath { - return true - } - } - - return false + return fieldPath[len(fieldPath)-1] == "metadata" } // getObjectMetaType returns the ObjectMeta type from the schema definitions From c4f1f6166af475d81e20d9ca5d3bccf014145da0 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 31 Jul 2025 10:27:55 +0200 Subject: [PATCH 5/6] json On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/exports_test.go | 8 --- gateway/schema/metadata_inference_test.go | 73 ----------------------- gateway/schema/schema.go | 44 +------------- 3 files changed, 2 insertions(+), 123 deletions(-) delete mode 100644 gateway/schema/metadata_inference_test.go diff --git a/gateway/schema/exports_test.go b/gateway/schema/exports_test.go index 2522b934..40c2aab4 100644 --- a/gateway/schema/exports_test.go +++ b/gateway/schema/exports_test.go @@ -21,11 +21,3 @@ func (g *Gateway) GenerateTypeNameForTest(typePrefix string, fieldPath []string) func SanitizeFieldNameForTest(name string) string { return sanitizeFieldName(name) } - -func (g *Gateway) ShouldInferAsObjectMetaForTest(fieldPath []string) bool { - return g.shouldInferAsObjectMeta(fieldPath) -} - -func (g *Gateway) GetObjectMetaTypeForTest() (interface{}, interface{}, error) { - return g.getObjectMetaType() -} diff --git a/gateway/schema/metadata_inference_test.go b/gateway/schema/metadata_inference_test.go deleted file mode 100644 index 0450d507..00000000 --- a/gateway/schema/metadata_inference_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package schema_test - -import ( - "testing" - - "github.com/openmfp/kubernetes-graphql-gateway/gateway/schema" -) - -func TestShouldInferAsObjectMeta(t *testing.T) { - g := schema.GetGatewayForTest(map[string]string{}) - - tests := []struct { - name string - fieldPath []string - expected bool - }{ - { - name: "one_level_deep_metadata", - fieldPath: []string{"metadata"}, - expected: true, - }, - { - name: "two_level_deep_metadata", - fieldPath: []string{"spec", "metadata"}, - expected: true, - }, - { - name: "three_level_deep_metadata", - fieldPath: []string{"spec", "apiExport", "metadata"}, - expected: true, - }, - - { - name: "non_metadata_field", - fieldPath: []string{"spec", "containers"}, - expected: false, - }, - { - name: "empty_field_path", - fieldPath: []string{}, - expected: false, - }, - { - name: "case_sensitive_metadata", - fieldPath: []string{"Metadata"}, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := g.ShouldInferAsObjectMetaForTest(tt.fieldPath) - if got != tt.expected { - t.Errorf("ShouldInferAsObjectMetaForTest(%v) = %v, want %v", tt.fieldPath, got, tt.expected) - } - }) - } -} - -func TestGetObjectMetaType_Fallback(t *testing.T) { - // Test that getObjectMetaType doesn't panic and returns something - g := schema.GetGatewayForTest(map[string]string{}) - - outputType, inputType, err := g.GetObjectMetaTypeForTest() - - if err != nil { - t.Errorf("GetObjectMetaTypeForTest() unexpected error: %v", err) - } - - if outputType == nil || inputType == nil { - t.Errorf("GetObjectMetaTypeForTest() should return non-nil types as fallback, got outputType=%v, inputType=%v", outputType, inputType) - } -} diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index 46ada63d..f9a516f0 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -453,48 +453,8 @@ func (g *Gateway) handleObjectFieldSpecType(fieldSpec spec.Schema, typePrefix st } } - // Check if this should be ObjectMeta - if g.shouldInferAsObjectMeta(fieldPath) { - return g.getObjectMetaType() - } - - // It's an empty object, use StringMap as fallback - return stringMapScalar, stringMapScalar, nil -} - -// shouldInferAsObjectMeta checks if a field path should be inferred as ObjectMeta -func (g *Gateway) shouldInferAsObjectMeta(fieldPath []string) bool { - if len(fieldPath) == 0 { - return false - } - - // Check if the last field in the path is "metadata" - return fieldPath[len(fieldPath)-1] == "metadata" -} - -// getObjectMetaType returns the ObjectMeta type from the schema definitions -func (g *Gateway) getObjectMetaType() (graphql.Output, graphql.Input, error) { - objectMetaKey := "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta" - - // First check if the ObjectMeta type is already cached (most common case) - if existingType, exists := g.typesCache[objectMetaKey]; exists { - existingInputType := g.inputTypesCache[objectMetaKey] - return existingType, existingInputType, nil - } - - // If not cached, try to generate it using the same key as normal $ref processing - if g.definitions != nil { - if objectMetaSchema, exists := g.definitions[objectMetaKey]; exists { - return g.handleObjectFieldSpecType(objectMetaSchema, objectMetaKey, []string{}, make(map[string]bool)) - } - } - - // Log warning when ObjectMeta definition is missing but expected - if g.log != nil { - g.log.Error().Msg("ObjectMeta definition not found in schema, falling back to StringMap") - } - - return stringMapScalar, stringMapScalar, nil + // It's an empty object, serialize as JSON string + return graphql.String, graphql.String, nil } func (g *Gateway) generateTypeName(typePrefix string, fieldPath []string) string { From d0a9097c3c6a0ce7f13d2b90090c537ec48a7c2f Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 31 Jul 2025 12:24:30 +0200 Subject: [PATCH 6/6] jsonScalar On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/exports_test.go | 1 + gateway/schema/scalars.go | 38 ++++++++++++++++++++++++++ gateway/schema/scalars_test.go | 49 ++++++++++++++++++++++++++++++++++ gateway/schema/schema.go | 2 +- 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/gateway/schema/exports_test.go b/gateway/schema/exports_test.go index 40c2aab4..9c099ccd 100644 --- a/gateway/schema/exports_test.go +++ b/gateway/schema/exports_test.go @@ -3,6 +3,7 @@ package schema import "k8s.io/apimachinery/pkg/runtime/schema" var StringMapScalarForTest = stringMapScalar +var JSONStringScalarForTest = jsonStringScalar func GetGatewayForTest(typeNameRegistry map[string]string) *Gateway { return &Gateway{ diff --git a/gateway/schema/scalars.go b/gateway/schema/scalars.go index 97e02063..88c2a419 100644 --- a/gateway/schema/scalars.go +++ b/gateway/schema/scalars.go @@ -1,6 +1,8 @@ package schema import ( + "encoding/json" + "github.com/graphql-go/graphql" "github.com/graphql-go/graphql/language/ast" ) @@ -34,3 +36,39 @@ var stringMapScalar = graphql.NewScalar(graphql.ScalarConfig{ } }, }) + +var jsonStringScalar = graphql.NewScalar(graphql.ScalarConfig{ + Name: "JSONString", + Description: "A JSON-serialized string representation of any object.", + Serialize: func(value interface{}) interface{} { + // Convert the value to JSON string + jsonBytes, err := json.Marshal(value) + if err != nil { + // Fallback to empty JSON object if marshaling fails + return "{}" + } + return string(jsonBytes) + }, + ParseValue: func(value interface{}) interface{} { + if str, ok := value.(string); ok { + var result interface{} + err := json.Unmarshal([]byte(str), &result) + if err != nil { + return nil // Invalid JSON + } + return result + } + return nil + }, + ParseLiteral: func(valueAST ast.Value) interface{} { + if value, ok := valueAST.(*ast.StringValue); ok { + var result interface{} + err := json.Unmarshal([]byte(value.Value), &result) + if err != nil { + return nil // Invalid JSON + } + return result + } + return nil + }, +}) diff --git a/gateway/schema/scalars_test.go b/gateway/schema/scalars_test.go index 1ab9616c..8f20a5ca 100644 --- a/gateway/schema/scalars_test.go +++ b/gateway/schema/scalars_test.go @@ -1,6 +1,7 @@ package schema_test import ( + "encoding/json" "reflect" "testing" @@ -153,3 +154,51 @@ func TestGenerateTypeName(t *testing.T) { }) } } + +func TestJSONStringScalar_ProperSerialization(t *testing.T) { + testObject := map[string]interface{}{ + "name": "example-config", + "namespace": "default", + "labels": map[string]string{ + "hello": "world", + }, + "annotations": map[string]string{ + "kcp.io/cluster": "root", + }, + } + + // Test the JSONString scalar serialization + result := schema.JSONStringScalarForTest.Serialize(testObject) + + if result == nil { + t.Fatal("JSONStringScalar.Serialize returned nil") + } + + resultStr, ok := result.(string) + if !ok { + t.Fatalf("JSONStringScalar.Serialize returned %T, expected string", result) + } + + // Verify it's valid JSON + var parsed map[string]interface{} + err := json.Unmarshal([]byte(resultStr), &parsed) + if err != nil { + t.Fatalf("Result is not valid JSON: %s\nResult: %s", err, resultStr) + } + + // Verify the content is preserved + if parsed["name"] != "example-config" { + t.Errorf("Name not preserved: got %v, want %v", parsed["name"], "example-config") + } + + if parsed["namespace"] != "default" { + t.Errorf("Namespace not preserved: got %v, want %v", parsed["namespace"], "default") + } + + // Verify it's NOT Go map format + if len(resultStr) > 10 && resultStr[:4] == "map[" { + t.Errorf("Result is in Go map format, not JSON: %s", resultStr) + } + + t.Logf("Proper JSON output: %s", resultStr) +} diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index f9a516f0..9085ec27 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -454,7 +454,7 @@ func (g *Gateway) handleObjectFieldSpecType(fieldSpec spec.Schema, typePrefix st } // It's an empty object, serialize as JSON string - return graphql.String, graphql.String, nil + return jsonStringScalar, jsonStringScalar, nil } func (g *Gateway) generateTypeName(typePrefix string, fieldPath []string) string {