From ad375ed5f5877b56251d279166413a5a98bd977d Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 10:24:07 +0200 Subject: [PATCH 01/22] feat: relations On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 120 ++++++++++ gateway/resolver/resolver.go | 24 +- gateway/schema/relations.go | 140 +++++++++++ gateway/schema/schema.go | 39 +-- listener/pkg/apischema/builder.go | 225 +++++++++++++++--- listener/pkg/apischema/crd_resolver.go | 2 + listener/pkg/apischema/relationships_test.go | 84 +++++++ .../helpers_rbac_authorization_k8s_io_test.go | 19 +- tests/gateway_test/relation_rbac_test.go | 113 +++++++++ 9 files changed, 707 insertions(+), 59 deletions(-) create mode 100644 gateway/resolver/relations.go create mode 100644 gateway/schema/relations.go create mode 100644 listener/pkg/apischema/relationships_test.go create mode 100644 tests/gateway_test/relation_rbac_test.go diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go new file mode 100644 index 00000000..1027b9e8 --- /dev/null +++ b/gateway/resolver/relations.go @@ -0,0 +1,120 @@ +package resolver + +import ( + "context" + + "github.com/graphql-go/graphql" + "golang.org/x/text/cases" + "golang.org/x/text/language" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// RelationResolver handles runtime resolution of relation fields +type RelationResolver struct { + service *Service +} + +// NewRelationResolver creates a new relation resolver +func NewRelationResolver(service *Service) *RelationResolver { + return &RelationResolver{ + service: service, + } +} + +// CreateResolver creates a GraphQL resolver for relation fields +func (rr *RelationResolver) CreateResolver(fieldName string) graphql.FieldResolveFn { + return func(p graphql.ResolveParams) (interface{}, error) { + parentObj, ok := p.Source.(map[string]interface{}) + if !ok { + return nil, nil + } + + refInfo := rr.extractReferenceInfo(parentObj, fieldName) + if refInfo.name == "" { + return nil, nil + } + + return rr.resolveReference(p.Context, refInfo.name, refInfo.namespace, refInfo.kind, refInfo.apiGroup) + } +} + +// referenceInfo holds extracted reference details +type referenceInfo struct { + name string + namespace string + kind string + apiGroup string +} + +// extractReferenceInfo extracts reference details from a *Ref object +func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]interface{}, fieldName string) referenceInfo { + name, _ := parentObj["name"].(string) + if name == "" { + return referenceInfo{} + } + + namespace, _ := parentObj["namespace"].(string) + apiGroup, _ := parentObj["apiGroup"].(string) + + kind, _ := parentObj["kind"].(string) + if kind == "" { + // Fallback: infer kind from field name (e.g., "role" -> "Role") + kind = cases.Title(language.English).String(fieldName) + } + + return referenceInfo{ + name: name, + namespace: namespace, + kind: kind, + apiGroup: apiGroup, + } +} + +// resolveReference fetches a referenced Kubernetes resource +func (rr *RelationResolver) resolveReference(ctx context.Context, name, namespace, kind, apiGroup string) (interface{}, error) { + versions := []string{"v1", "v1beta1", "v1alpha1"} + + for _, version := range versions { + if obj := rr.tryFetchResource(ctx, name, namespace, kind, apiGroup, version); obj != nil { + return obj, nil + } + } + + return nil, nil +} + +// tryFetchResource attempts to fetch a Kubernetes resource with the given parameters +func (rr *RelationResolver) tryFetchResource(ctx context.Context, name, namespace, kind, apiGroup, version string) map[string]interface{} { + gvk := schema.GroupVersionKind{ + Group: apiGroup, + Version: version, + Kind: kind, + } + + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + + key := client.ObjectKey{Name: name} + if namespace != "" { + key.Namespace = namespace + } + + if err := rr.service.runtimeClient.Get(ctx, key, obj); err == nil { + return obj.Object + } + + return nil +} + +// GetSupportedVersions returns the list of API versions to try for resource resolution +func (rr *RelationResolver) GetSupportedVersions() []string { + return []string{"v1", "v1beta1", "v1alpha1"} +} + +// SetSupportedVersions allows customizing the API versions to try (for future extensibility) +func (rr *RelationResolver) SetSupportedVersions(versions []string) { + // Future: Store in resolver state for customization + // For now, this is a placeholder for extensibility +} diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index a99e38d8..02f2d091 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -30,6 +30,8 @@ type Provider interface { CustomQueriesProvider CommonResolver() graphql.FieldResolveFn SanitizeGroupName(string) string + RuntimeClient() client.WithWatch + RelationResolver(fieldName string) graphql.FieldResolveFn } type CrudProvider interface { @@ -50,16 +52,22 @@ type CustomQueriesProvider interface { type Service struct { log *logger.Logger // groupNames stores relation between sanitized group names and original group names that are used in the Kubernetes API - groupNames map[string]string // map[sanitizedGroupName]originalGroupName - runtimeClient client.WithWatch + groupNames map[string]string // map[sanitizedGroupName]originalGroupName + runtimeClient client.WithWatch + relationResolver *RelationResolver } func New(log *logger.Logger, runtimeClient client.WithWatch) *Service { - return &Service{ + s := &Service{ log: log, groupNames: make(map[string]string), runtimeClient: runtimeClient, } + + // Initialize the relation resolver + s.relationResolver = NewRelationResolver(s) + + return s } // ListItems returns a GraphQL CommonResolver function that lists Kubernetes resources of the given GroupVersionKind. @@ -456,3 +464,13 @@ func compareNumbers[T int64 | float64](a, b T) int { return 0 } } + +// RuntimeClient returns the runtime client for use in relationship resolution +func (r *Service) RuntimeClient() client.WithWatch { + return r.runtimeClient +} + +// RelationResolver creates a GraphQL resolver for relation fields +func (r *Service) RelationResolver(fieldName string) graphql.FieldResolveFn { + return r.relationResolver.CreateResolver(fieldName) +} diff --git a/gateway/schema/relations.go b/gateway/schema/relations.go new file mode 100644 index 00000000..8defdb0b --- /dev/null +++ b/gateway/schema/relations.go @@ -0,0 +1,140 @@ +package schema + +import ( + "strings" + + "golang.org/x/text/cases" + "golang.org/x/text/language" + + "github.com/go-openapi/spec" + "github.com/graphql-go/graphql" +) + +// RelationEnhancer handles schema enhancement for relation fields +type RelationEnhancer struct { + gateway *Gateway +} + +// NewRelationEnhancer creates a new relation enhancer +func NewRelationEnhancer(gateway *Gateway) *RelationEnhancer { + return &RelationEnhancer{ + gateway: gateway, + } +} + +// AddRelationFields adds relation fields to schemas that contain *Ref fields +func (re *RelationEnhancer) AddRelationFields(fields graphql.Fields, properties map[string]spec.Schema) { + for fieldName := range properties { + if !strings.HasSuffix(fieldName, "Ref") { + continue + } + + baseName := strings.TrimSuffix(fieldName, "Ref") + sanitizedFieldName := sanitizeFieldName(fieldName) + + refField, exists := fields[sanitizedFieldName] + if !exists { + continue + } + + enhancedType := re.enhanceRefTypeWithRelation(refField.Type, baseName) + if enhancedType == nil { + continue + } + + fields[sanitizedFieldName] = &graphql.Field{ + Type: enhancedType, + } + } +} + +// enhanceRefTypeWithRelation adds a relation field to a *Ref object type +func (re *RelationEnhancer) enhanceRefTypeWithRelation(originalType graphql.Output, baseName string) graphql.Output { + objType, ok := originalType.(*graphql.Object) + if !ok { + return originalType + } + + cacheKey := objType.Name() + "_" + baseName + "_Enhanced" + if enhancedType, exists := re.gateway.enhancedTypesCache[cacheKey]; exists { + return enhancedType + } + + enhancedFields := re.copyOriginalFields(objType.Fields()) + re.addRelationField(enhancedFields, baseName) + + enhancedType := graphql.NewObject(graphql.ObjectConfig{ + Name: sanitizeFieldName(cacheKey), + Fields: enhancedFields, + }) + + re.gateway.enhancedTypesCache[cacheKey] = enhancedType + return enhancedType +} + +// copyOriginalFields converts FieldDefinition to Field for reuse +func (re *RelationEnhancer) copyOriginalFields(originalFieldDefs graphql.FieldDefinitionMap) graphql.Fields { + enhancedFields := make(graphql.Fields, len(originalFieldDefs)) + for fieldName, fieldDef := range originalFieldDefs { + enhancedFields[fieldName] = &graphql.Field{ + Type: fieldDef.Type, + Description: fieldDef.Description, + Resolve: fieldDef.Resolve, + } + } + return enhancedFields +} + +// addRelationField adds a single relation field to the enhanced fields +func (re *RelationEnhancer) addRelationField(enhancedFields graphql.Fields, baseName string) { + targetType := re.findRelationTargetType(baseName) + if targetType == nil { + return + } + + sanitizedBaseName := sanitizeFieldName(baseName) + enhancedFields[sanitizedBaseName] = &graphql.Field{ + Type: targetType, + Resolve: re.gateway.resolver.RelationResolver(baseName), + } +} + +// findRelationTargetType finds the GraphQL type for a relation target +func (re *RelationEnhancer) findRelationTargetType(baseName string) graphql.Output { + targetKind := cases.Title(language.English).String(baseName) + + for defKey, defSchema := range re.gateway.definitions { + if re.matchesTargetKind(defSchema, targetKind) { + if existingType, exists := re.gateway.typesCache[defKey]; exists { + return existingType + } + + if fieldType, _, err := re.gateway.convertSwaggerTypeToGraphQL(defSchema, defKey, []string{}, make(map[string]bool)); err == nil { + return fieldType + } + } + } + + return graphql.String +} + +// matchesTargetKind checks if a schema definition matches the target kind +func (re *RelationEnhancer) matchesTargetKind(defSchema spec.Schema, targetKind string) bool { + gvkExt, ok := defSchema.Extensions["x-kubernetes-group-version-kind"] + if !ok { + return false + } + + gvkSlice, ok := gvkExt.([]any) + if !ok || len(gvkSlice) == 0 { + return false + } + + gvkMap, ok := gvkSlice[0].(map[string]any) + if !ok { + return false + } + + kind, ok := gvkMap["kind"].(string) + return ok && kind == targetKind +} diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index 9085ec27..edaac1b7 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -22,16 +22,14 @@ type Provider interface { } type Gateway struct { - log *logger.Logger - resolver resolver.Provider - graphqlSchema graphql.Schema - - definitions spec.Definitions - - // typesCache stores generated GraphQL object types(fields) to prevent redundant repeated generation. - typesCache map[string]*graphql.Object - // inputTypesCache stores generated GraphQL input object types(input fields) to prevent redundant repeated generation. - inputTypesCache map[string]*graphql.InputObject + log *logger.Logger + resolver resolver.Provider + graphqlSchema graphql.Schema + definitions spec.Definitions + typesCache map[string]*graphql.Object + inputTypesCache map[string]*graphql.InputObject + enhancedTypesCache map[string]*graphql.Object // Cache for enhanced *Ref types + relationEnhancer *RelationEnhancer // Prevents naming conflict in case of the same Kind name in different groups/versions typeNameRegistry map[string]string // map[Kind]GroupVersion @@ -41,15 +39,19 @@ type Gateway struct { func New(log *logger.Logger, definitions spec.Definitions, resolverProvider resolver.Provider) (*Gateway, error) { g := &Gateway{ - log: log, - resolver: resolverProvider, - definitions: definitions, - typesCache: make(map[string]*graphql.Object), - inputTypesCache: make(map[string]*graphql.InputObject), - typeNameRegistry: make(map[string]string), - typeByCategory: make(map[string][]resolver.TypeByCategory), + log: log, + resolver: resolverProvider, + definitions: definitions, + typesCache: make(map[string]*graphql.Object), + inputTypesCache: make(map[string]*graphql.InputObject), + enhancedTypesCache: make(map[string]*graphql.Object), + typeNameRegistry: make(map[string]string), + typeByCategory: make(map[string][]resolver.TypeByCategory), } + // Initialize the relation enhancer after gateway is created + g.relationEnhancer = NewRelationEnhancer(g) + err := g.generateGraphqlSchema() return g, err @@ -336,6 +338,9 @@ func (g *Gateway) generateGraphQLFields(resourceScheme *spec.Schema, typePrefix } } + // Add relation fields for any *Ref fields in this schema + g.relationEnhancer.AddRelationFields(fields, resourceScheme.Properties) + return fields, inputFields, nil } diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 81ef41df..ca9f0f37 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -30,18 +30,29 @@ var ( ErrCRDNoVersions = errors.New("CRD has no versions defined") ErrMarshalGVK = errors.New("failed to marshal GVK extension") ErrUnmarshalGVK = errors.New("failed to unmarshal GVK extension") + ErrBuildKindRegistry = errors.New("failed to build kind registry") ) type SchemaBuilder struct { - schemas map[string]*spec.Schema - err *multierror.Error - log *logger.Logger + schemas map[string]*spec.Schema + err *multierror.Error + log *logger.Logger + kindRegistry map[string][]ResourceInfo +} + +// ResourceInfo holds information about a resource for relationship resolution +type ResourceInfo struct { + Group string + Version string + Kind string + SchemaKey string } func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logger.Logger) *SchemaBuilder { b := &SchemaBuilder{ - schemas: make(map[string]*spec.Schema), - log: log, + schemas: make(map[string]*spec.Schema), + kindRegistry: make(map[string][]ResourceInfo), + log: log, } apiv3Paths, err := oc.Paths() @@ -107,72 +118,210 @@ func (b *SchemaBuilder) WithScope(rm meta.RESTMapper) *SchemaBuilder { Str("group", gvks[0].Group). Str("version", gvks[0].Version). Str("kind", gvks[0].Kind). - Msg("failed to determine if GVK is namespaced") + Msg("failed to get namespaced info for GVK") continue } if namespaced { - schema.VendorExtensible.AddExtension(common.ScopeExtensionKey, apiextensionsv1.NamespaceScoped) + if schema.VendorExtensible.Extensions == nil { + schema.VendorExtensible.Extensions = map[string]any{} + } + schema.VendorExtensible.Extensions[common.ScopeExtensionKey] = apiextensionsv1.NamespaceScoped } else { - schema.VendorExtensible.AddExtension(common.ScopeExtensionKey, apiextensionsv1.ClusterScoped) + if schema.VendorExtensible.Extensions == nil { + schema.VendorExtensible.Extensions = map[string]any{} + } + schema.VendorExtensible.Extensions[common.ScopeExtensionKey] = apiextensionsv1.ClusterScoped } - } - return b } func (b *SchemaBuilder) WithCRDCategories(crd *apiextensionsv1.CustomResourceDefinition) *SchemaBuilder { - categories := crd.Spec.Names.Categories - if len(categories) == 0 { - return b - } - gvk, err := getCRDGroupVersionKind(crd.Spec) - if err != nil { - b.err = multierror.Append(b.err, errors.Join(ErrGetCRDGVK, err)) + if crd == nil { return b } - schema, ok := b.schemas[getOpenAPISchemaKey(*gvk)] - if !ok { + gkv, err := getCRDGroupVersionKind(crd.Spec) + if err != nil { + b.err = multierror.Append(b.err, ErrGetCRDGVK) return b } - schema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, categories) + for _, v := range crd.Spec.Versions { + resourceKey := getOpenAPISchemaKey(metav1.GroupVersionKind{Group: gkv.Group, Version: v.Name, Kind: gkv.Kind}) + resourceSchema, ok := b.schemas[resourceKey] + if !ok { + continue + } + if len(crd.Spec.Names.Categories) == 0 { + b.log.Debug().Str("resource", resourceKey).Msg("no categories provided for CRD kind") + continue + } + if resourceSchema.VendorExtensible.Extensions == nil { + resourceSchema.VendorExtensible.Extensions = map[string]any{} + } + resourceSchema.VendorExtensible.Extensions[common.CategoriesExtensionKey] = crd.Spec.Names.Categories + b.schemas[resourceKey] = resourceSchema + } return b } func (b *SchemaBuilder) WithApiResourceCategories(list []*metav1.APIResourceList) *SchemaBuilder { + if len(list) == 0 { + return b + } + for _, apiResourceList := range list { + gv, err := runtimeSchema.ParseGroupVersion(apiResourceList.GroupVersion) + if err != nil { + b.err = multierror.Append(b.err, errors.Join(ErrParseGroupVersion, err)) + continue + } for _, apiResource := range apiResourceList.APIResources { if apiResource.Categories == nil { continue } - - gv, err := runtimeSchema.ParseGroupVersion(apiResourceList.GroupVersion) - if err != nil { - b.err = multierror.Append(b.err, errors.Join(ErrParseGroupVersion, err)) + gvk := metav1.GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: apiResource.Kind} + resourceKey := getOpenAPISchemaKey(gvk) + resourceSchema, ok := b.schemas[resourceKey] + if !ok { continue } - gvk := metav1.GroupVersionKind{ - Group: gv.Group, - Version: gv.Version, - Kind: apiResource.Kind, + if resourceSchema.VendorExtensible.Extensions == nil { + resourceSchema.VendorExtensible.Extensions = map[string]any{} } + resourceSchema.VendorExtensible.Extensions[common.CategoriesExtensionKey] = apiResource.Categories + b.schemas[resourceKey] = resourceSchema + } + } + return b +} - schema, ok := b.schemas[getOpenAPISchemaKey(gvk)] - if !ok { - continue - } +// WithRelationships adds relationship fields to schemas that have *Ref fields +func (b *SchemaBuilder) WithRelationships() *SchemaBuilder { + // Build kind registry first + b.buildKindRegistry() - schema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, apiResource.Categories) - } + // Expand relationships in all schemas + b.log.Info().Int("kindRegistrySize", len(b.kindRegistry)).Msg("Starting relationship expansion") + for schemaKey, schema := range b.schemas { + b.log.Debug().Str("schemaKey", schemaKey).Msg("Processing schema for relationships") + b.expandRelationships(schema) } return b } +// buildKindRegistry builds a map of kind names to available resource types +func (b *SchemaBuilder) buildKindRegistry() { + for schemaKey, schema := range b.schemas { + // Extract GVK from schema + if schema.VendorExtensible.Extensions == nil { + continue + } + + gvksVal, ok := schema.VendorExtensible.Extensions[common.GVKExtensionKey] + if !ok { + continue + } + + jsonBytes, err := json.Marshal(gvksVal) + if err != nil { + b.log.Debug().Err(err).Str("schemaKey", schemaKey).Msg("failed to marshal GVK") + continue + } + + var gvks []*GroupVersionKind + if err := json.Unmarshal(jsonBytes, &gvks); err != nil { + b.log.Debug().Err(err).Str("schemaKey", schemaKey).Msg("failed to unmarshal GVK") + continue + } + + if len(gvks) != 1 { + continue + } + + gvk := gvks[0] + + // Add to kind registry + resourceInfo := ResourceInfo{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + SchemaKey: schemaKey, + } + + // Index by lowercase kind name for consistent lookup + b.kindRegistry[strings.ToLower(gvk.Kind)] = append(b.kindRegistry[strings.ToLower(gvk.Kind)], resourceInfo) + } + + b.log.Debug().Int("kindCount", len(b.kindRegistry)).Msg("built kind registry for relationships") +} + +// expandRelationships detects fields ending with 'Ref' and adds corresponding relationship fields +func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { + if schema.Properties == nil { + return + } + + // Create a copy of properties to avoid modifying while iterating + originalProps := make(map[string]spec.Schema) + for k, v := range schema.Properties { + originalProps[k] = v + } + + for propName := range originalProps { + if strings.HasSuffix(propName, "Ref") && propName != "Ref" { + // Extract the base kind (e.g., "role" from "roleRef") + baseKind := strings.TrimSuffix(propName, "Ref") + b.log.Debug().Str("propName", propName).Str("baseKind", baseKind).Msg("Found Ref field") + + // Find the first matching resource type for this kind + lookupKey := strings.ToLower(baseKind) + b.log.Debug().Str("lookupKey", lookupKey).Msg("Looking up in kind registry") + if resourceTypes, exists := b.kindRegistry[lookupKey]; exists && len(resourceTypes) > 0 { + b.log.Debug().Str("lookupKey", lookupKey).Int("resourceCount", len(resourceTypes)).Msg("Found matching resources") + // Use the first matching resource type + targetResource := resourceTypes[0] + + // Generate field name (e.g., "role" for "roleRef") + fieldName := strings.ToLower(baseKind) + + // Add the relationship field + if _, exists := schema.Properties[fieldName]; !exists { + // Create a reference to the target schema + refSchema := spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef(fmt.Sprintf("#/definitions/%s.%s.%s", + targetResource.Group, targetResource.Version, targetResource.Kind)), + }, + } + schema.Properties[fieldName] = refSchema + + b.log.Info(). + Str("sourceField", propName). + Str("targetField", fieldName). + Str("targetKind", targetResource.Kind). + Str("targetGroup", targetResource.Group). + Msg("Added relationship field") + } + } else { + b.log.Debug().Str("lookupKey", lookupKey).Msg("No matching resources found in kind registry") + } + } + } + + // Recursively process nested objects and write back modifications + for key, prop := range schema.Properties { + if prop.Type.Contains("object") && prop.Properties != nil { + b.expandRelationships(&prop) + schema.Properties[key] = prop + } + } +} + func (b *SchemaBuilder) Complete() ([]byte, error) { v3JSON, err := json.Marshal(&schemaResponse{ Components: schemasComponentsWrapper{ @@ -180,18 +329,18 @@ func (b *SchemaBuilder) Complete() ([]byte, error) { }, }) if err != nil { - b.err = multierror.Append(b.err, errors.Join(ErrMarshalOpenAPISchema, err)) - return nil, b.err + return nil, errors.Join(ErrMarshalOpenAPISchema, err) } + v2JSON, err := ConvertJSON(v3JSON) if err != nil { - b.err = multierror.Append(b.err, errors.Join(ErrConvertOpenAPISchema, err)) - return nil, b.err + return nil, errors.Join(ErrConvertOpenAPISchema, err) } return v2JSON, nil } +// getOpenAPISchemaKey creates the key that kubernetes uses in its OpenAPI Definitions func getOpenAPISchemaKey(gvk metav1.GroupVersionKind) string { // we need to inverse group to match the runtimeSchema key(io.openmfp.core.v1alpha1.Account) parts := strings.Split(gvk.Group, ".") diff --git a/listener/pkg/apischema/crd_resolver.go b/listener/pkg/apischema/crd_resolver.go index bae15b35..a83d4469 100644 --- a/listener/pkg/apischema/crd_resolver.go +++ b/listener/pkg/apischema/crd_resolver.go @@ -77,6 +77,7 @@ func (cr *CRDResolver) ResolveApiSchema(crd *apiextensionsv1.CustomResourceDefin result, err := NewSchemaBuilder(cr.OpenAPIV3(), preferredApiGroups, cr.log). WithScope(cr.RESTMapper). WithCRDCategories(crd). + WithRelationships(). Complete() if err != nil { @@ -207,6 +208,7 @@ func (cr *CRDResolver) resolveSchema(dc discovery.DiscoveryInterface, rm meta.RE result, err := NewSchemaBuilder(dc.OpenAPIV3(), preferredApiGroups, cr.log). WithScope(rm). WithApiResourceCategories(apiResList). + WithRelationships(). Complete() if err != nil { diff --git a/listener/pkg/apischema/relationships_test.go b/listener/pkg/apischema/relationships_test.go new file mode 100644 index 00000000..d117508c --- /dev/null +++ b/listener/pkg/apischema/relationships_test.go @@ -0,0 +1,84 @@ +package apischema_test + +import ( + "testing" + + "github.com/openmfp/golang-commons/logger/testlogger" + apischema "github.com/openmfp/kubernetes-graphql-gateway/listener/pkg/apischema" + apimocks "github.com/openmfp/kubernetes-graphql-gateway/listener/pkg/apischema/mocks" + "github.com/stretchr/testify/assert" + "k8s.io/client-go/openapi" + "k8s.io/kube-openapi/pkg/validation/spec" +) + +// helper constructs a schema with x-kubernetes-group-version-kind +func schemaWithGVK(group, version, kind string) *spec.Schema { + return &spec.Schema{ + VendorExtensible: spec.VendorExtensible{Extensions: map[string]interface{}{ + "x-kubernetes-group-version-kind": []map[string]string{{ + "group": group, + "version": version, + "kind": kind, + }}, + }}, + } +} + +func Test_with_relationships_adds_single_target_field(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // definitions contain a target kind Role in group g/v1 + roleKey := "g.v1.Role" + roleSchema := schemaWithGVK("g", "v1", "Role") + + // source schema that has roleRef + sourceKey := "g2.v1.Binding" + sourceSchema := &spec.Schema{SchemaProps: spec.SchemaProps{Properties: map[string]spec.Schema{ + "roleRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + }}} + + b.SetSchemas(map[string]*spec.Schema{ + roleKey: roleSchema, + sourceKey: sourceSchema, + }) + + b.WithRelationships() + + // Expect that role field was added referencing the Role definition + added, ok := b.GetSchemas()[sourceKey].Properties["role"] + assert.True(t, ok, "expected relationship field 'role' to be added") + assert.True(t, added.Ref.GetURL() != nil, "expected $ref on relationship field") + assert.Contains(t, added.Ref.String(), "#/definitions/g.v1.Role") +} + +func Test_build_kind_registry_lowercases_keys_and_picks_first(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Two schemas with same Kind different groups/versions - first should win + first := schemaWithGVK("a.example", "v1", "Thing") + second := schemaWithGVK("b.example", "v1", "Thing") + b.SetSchemas(map[string]*spec.Schema{ + "a.example.v1.Thing": first, + "b.example.v1.Thing": second, + "c.other.v1.Other": schemaWithGVK("c.other", "v1", "Other"), + }) + + b.WithRelationships() // indirectly builds the registry + + // validate lowercase key exists and contains both, but expansion uses first (covered by previous test) + // we assert the registry was built by triggering another schema that references thingRef + sRef := &spec.Schema{SchemaProps: spec.SchemaProps{Properties: map[string]spec.Schema{ + "thingRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + }}} + b.GetSchemas()["x.v1.HasThing"] = sRef + + b.WithRelationships() + added, ok := b.GetSchemas()["x.v1.HasThing"].Properties["thing"] + assert.True(t, ok, "expected relationship field 'thing'") + // ensure it referenced the first group + assert.Contains(t, added.Ref.String(), "#/definitions/a.example.v1.Thing") +} diff --git a/tests/gateway_test/helpers_rbac_authorization_k8s_io_test.go b/tests/gateway_test/helpers_rbac_authorization_k8s_io_test.go index 5ecd3f21..33843c72 100644 --- a/tests/gateway_test/helpers_rbac_authorization_k8s_io_test.go +++ b/tests/gateway_test/helpers_rbac_authorization_k8s_io_test.go @@ -1,13 +1,30 @@ package gateway_test type RbacAuthorizationK8sIO struct { - ClusterRole *ClusterRole `json:"ClusterRole,omitempty"` + ClusterRole *ClusterRole `json:"ClusterRole,omitempty"` + ClusterRoleBinding *ClusterRoleBinding `json:"ClusterRoleBinding,omitempty"` } type ClusterRole struct { Metadata metadata `json:"metadata"` } +type ClusterRoleBinding struct { + Metadata metadata `json:"metadata"` + RoleRef roleRef `json:"roleRef"` +} + +type roleRef struct { + Name string `json:"name"` + Kind string `json:"kind"` + APIGroup string `json:"apiGroup"` + Role crMeta `json:"role"` +} + +type crMeta struct { + Metadata metadata `json:"metadata"` +} + func CreateClusterRoleMutation() string { return `mutation { rbac_authorization_k8s_io { diff --git a/tests/gateway_test/relation_rbac_test.go b/tests/gateway_test/relation_rbac_test.go new file mode 100644 index 00000000..967af816 --- /dev/null +++ b/tests/gateway_test/relation_rbac_test.go @@ -0,0 +1,113 @@ +package gateway_test + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "path/filepath" + + "github.com/stretchr/testify/require" +) + +// Test_relation_clusterrolebinding_role_ref mirrors pod test style: creates schema file per workspace, +// creates a ClusterRole and ClusterRoleBinding via GraphQL, then queries roleRef.role to ensure relation resolution. +func (suite *CommonTestSuite) Test_relation_clusterrolebinding_role_ref() { + workspaceName := "relationsWorkspace" + + require.NoError(suite.T(), suite.writeToFileWithClusterMetadata( + filepath.Join("testdata", "kubernetes"), + filepath.Join(suite.appCfg.OpenApiDefinitionsPath, workspaceName), + )) + + url := fmt.Sprintf("%s/%s/graphql", suite.server.URL, workspaceName) + + // Create ClusterRole + statusCode, body := suite.doRawGraphQL(url, createClusterRoleForRelationMutation()) + require.Equal(suite.T(), http.StatusOK, statusCode) + require.Nil(suite.T(), body["errors"]) + + // Create ClusterRoleBinding referencing the ClusterRole + statusCode, body = suite.doRawGraphQL(url, createClusterRoleBindingForRelationMutation()) + require.Equal(suite.T(), http.StatusOK, statusCode) + require.Nil(suite.T(), body["errors"]) + + // Query ClusterRoleBinding and expand roleRef.role + statusCode, body = suite.doRawGraphQL(url, getClusterRoleBindingWithRoleQuery()) + require.Equal(suite.T(), http.StatusOK, statusCode) + require.Nil(suite.T(), body["errors"]) + + // Extract nested role name from generic map + data, _ := body["data"].(map[string]interface{}) + rbac, _ := data["rbac_authorization_k8s_io"].(map[string]interface{}) + crb, _ := rbac["ClusterRoleBinding"].(map[string]interface{}) + roleRef, _ := crb["roleRef"].(map[string]interface{}) + role, _ := roleRef["role"].(map[string]interface{}) + metadata, _ := role["metadata"].(map[string]interface{}) + name, _ := metadata["name"].(string) + require.Equal(suite.T(), "test-cluster-role-rel", name) +} + +// local helper mirroring helpers_test.go but returning generic body +func (suite *CommonTestSuite) doRawGraphQL(url, query string) (int, map[string]interface{}) { + reqBody := map[string]string{"query": query} + buf, _ := json.Marshal(reqBody) + req, _ := http.NewRequest("POST", url, bytes.NewReader(buf)) + req.Header.Set("Content-Type", "application/json") + // add auth token used by suite + if suite.staticToken != "" { + req.Header.Set("Authorization", "Bearer "+suite.staticToken) + } + resp, err := http.DefaultClient.Do(req) + require.NoError(suite.T(), err) + defer resp.Body.Close() + var body map[string]interface{} + dec := json.NewDecoder(resp.Body) + require.NoError(suite.T(), dec.Decode(&body)) + return resp.StatusCode, body +} + +// GraphQL payloads +func createClusterRoleForRelationMutation() string { + return `mutation { + rbac_authorization_k8s_io { + createClusterRole( + object: { + metadata: { name: "test-cluster-role-rel" } + rules: [{ apiGroups:[""], resources:["pods"], verbs:["get","list"] }] + } + ) { metadata { name } } + } +}` +} + +func createClusterRoleBindingForRelationMutation() string { + return `mutation { + rbac_authorization_k8s_io { + createClusterRoleBinding( + object: { + metadata: { name: "test-crb-rel" } + roleRef: { + apiGroup: "rbac.authorization.k8s.io" + kind: "ClusterRole" + name: "test-cluster-role-rel" + } + subjects: [] + } + ) { metadata { name } } + } +}` +} + +func getClusterRoleBindingWithRoleQuery() string { + return `{ + rbac_authorization_k8s_io { + ClusterRoleBinding(name: "test-crb-rel") { + roleRef { + name kind apiGroup + role { metadata { name } } + } + } + } +}` +} From 20546e2189f6b3703db1eee8d14bb50179acf6cb Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 13:23:00 +0200 Subject: [PATCH 02/22] removed hardcoded versions On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 53 ++++++++++++----------------------- gateway/resolver/resolver.go | 50 ++++++++++++++++----------------- gateway/schema/relations.go | 31 ++++++++++++++------ 3 files changed, 64 insertions(+), 70 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index 1027b9e8..5239cd4f 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -24,7 +24,7 @@ func NewRelationResolver(service *Service) *RelationResolver { } // CreateResolver creates a GraphQL resolver for relation fields -func (rr *RelationResolver) CreateResolver(fieldName string) graphql.FieldResolveFn { +func (rr *RelationResolver) CreateResolver(fieldName string, targetGVK schema.GroupVersionKind) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { parentObj, ok := p.Source.(map[string]interface{}) if !ok { @@ -36,7 +36,7 @@ func (rr *RelationResolver) CreateResolver(fieldName string) graphql.FieldResolv return nil, nil } - return rr.resolveReference(p.Context, refInfo.name, refInfo.namespace, refInfo.kind, refInfo.apiGroup) + return rr.resolveReference(p.Context, refInfo, targetGVK) } } @@ -72,49 +72,32 @@ func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]interface{ } } -// resolveReference fetches a referenced Kubernetes resource -func (rr *RelationResolver) resolveReference(ctx context.Context, name, namespace, kind, apiGroup string) (interface{}, error) { - versions := []string{"v1", "v1beta1", "v1alpha1"} +// resolveReference fetches a referenced Kubernetes resource using provided target GVK +func (rr *RelationResolver) resolveReference(ctx context.Context, ref referenceInfo, targetGVK schema.GroupVersionKind) (interface{}, error) { + gvk := targetGVK - for _, version := range versions { - if obj := rr.tryFetchResource(ctx, name, namespace, kind, apiGroup, version); obj != nil { - return obj, nil - } + // Allow overrides from the reference object if specified + if ref.apiGroup != "" { + gvk.Group = ref.apiGroup } - - return nil, nil -} - -// tryFetchResource attempts to fetch a Kubernetes resource with the given parameters -func (rr *RelationResolver) tryFetchResource(ctx context.Context, name, namespace, kind, apiGroup, version string) map[string]interface{} { - gvk := schema.GroupVersionKind{ - Group: apiGroup, - Version: version, - Kind: kind, + if ref.kind != "" { + gvk.Kind = ref.kind } + // Convert sanitized group to original before calling the client + gvk.Group = rr.service.getOriginalGroupName(gvk.Group) + obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(gvk) - key := client.ObjectKey{Name: name} - if namespace != "" { - key.Namespace = namespace + key := client.ObjectKey{Name: ref.name} + if ref.namespace != "" { + key.Namespace = ref.namespace } if err := rr.service.runtimeClient.Get(ctx, key, obj); err == nil { - return obj.Object + return obj.Object, nil } - return nil -} - -// GetSupportedVersions returns the list of API versions to try for resource resolution -func (rr *RelationResolver) GetSupportedVersions() []string { - return []string{"v1", "v1beta1", "v1alpha1"} -} - -// SetSupportedVersions allows customizing the API versions to try (for future extensibility) -func (rr *RelationResolver) SetSupportedVersions(versions []string) { - // Future: Store in resolver state for customization - // For now, this is a placeholder for extensibility + return nil, nil } diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index 02f2d091..11a2f079 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/graphql-go/graphql" - pkgErrors "github.com/pkg/errors" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -31,7 +30,7 @@ type Provider interface { CommonResolver() graphql.FieldResolveFn SanitizeGroupName(string) string RuntimeClient() client.WithWatch - RelationResolver(fieldName string) graphql.FieldResolveFn + RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn } type CrudProvider interface { @@ -90,21 +89,11 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) log = r.log } - // Create an unstructured list to hold the results + // Create a list of unstructured objects to hold the results list := &unstructured.UnstructuredList{} - list.SetGroupVersionKind(gvk) + list.SetGroupVersionKind(schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind + "List"}) var opts []client.ListOption - // Handle label selector argument - if labelSelector, ok := p.Args[LabelSelectorArg].(string); ok && labelSelector != "" { - selector, err := labels.Parse(labelSelector) - if err != nil { - log.Error().Err(err).Str(LabelSelectorArg, labelSelector).Msg("Unable to parse given label selector") - return nil, err - } - opts = append(opts, client.MatchingLabelsSelector{Selector: selector}) - } - if isResourceNamespaceScoped(scope) { namespace, err := getStringArg(p.Args, NamespaceArg, false) if err != nil { @@ -115,25 +104,34 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) } } - if err = r.runtimeClient.List(ctx, list, opts...); err != nil { - log.Error().Err(err).Msg("Unable to list objects") - return nil, pkgErrors.Wrap(err, "unable to list objects") + if val, ok := p.Args[LabelSelectorArg].(string); ok && val != "" { + selector, err := labels.Parse(val) + if err != nil { + log.Error().Err(err).Str(LabelSelectorArg, val).Msg("Unable to parse given label selector") + return nil, err + } + opts = append(opts, client.MatchingLabelsSelector{Selector: selector}) } - sortBy, err := getStringArg(p.Args, SortByArg, false) - if err != nil { + if err = r.runtimeClient.List(ctx, list, opts...); err != nil { + log.Error().Err(err).Str("scope", string(scope)).Msg("Unable to list objects") return nil, err } - err = validateSortBy(list.Items, sortBy) + sortBy, err := getStringArg(p.Args, SortByArg, false) if err != nil { - log.Error().Err(err).Str(SortByArg, sortBy).Msg("Invalid sortBy field path") return nil, err } - sort.Slice(list.Items, func(i, j int) bool { - return compareUnstructured(list.Items[i], list.Items[j], sortBy) < 0 - }) + if sortBy != "" { + if err := validateSortBy(list.Items, sortBy); err != nil { + log.Error().Err(err).Str(SortByArg, sortBy).Msg("Invalid sortBy field path") + return nil, err + } + sort.Slice(list.Items, func(i, j int) bool { + return compareUnstructured(list.Items[i], list.Items[j], sortBy) < 0 + }) + } items := make([]map[string]any, len(list.Items)) for i, item := range list.Items { @@ -471,6 +469,6 @@ func (r *Service) RuntimeClient() client.WithWatch { } // RelationResolver creates a GraphQL resolver for relation fields -func (r *Service) RelationResolver(fieldName string) graphql.FieldResolveFn { - return r.relationResolver.CreateResolver(fieldName) +func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { + return r.relationResolver.CreateResolver(fieldName, gvk) } diff --git a/gateway/schema/relations.go b/gateway/schema/relations.go index 8defdb0b..cd18d94d 100644 --- a/gateway/schema/relations.go +++ b/gateway/schema/relations.go @@ -8,6 +8,7 @@ import ( "github.com/go-openapi/spec" "github.com/graphql-go/graphql" + "k8s.io/apimachinery/pkg/runtime/schema" ) // RelationEnhancer handles schema enhancement for relation fields @@ -87,35 +88,47 @@ func (re *RelationEnhancer) copyOriginalFields(originalFieldDefs graphql.FieldDe // addRelationField adds a single relation field to the enhanced fields func (re *RelationEnhancer) addRelationField(enhancedFields graphql.Fields, baseName string) { - targetType := re.findRelationTargetType(baseName) - if targetType == nil { + targetType, targetGVK, ok := re.findRelationTarget(baseName) + if !ok { return } sanitizedBaseName := sanitizeFieldName(baseName) enhancedFields[sanitizedBaseName] = &graphql.Field{ Type: targetType, - Resolve: re.gateway.resolver.RelationResolver(baseName), + Resolve: re.gateway.resolver.RelationResolver(baseName, *targetGVK), } } -// findRelationTargetType finds the GraphQL type for a relation target -func (re *RelationEnhancer) findRelationTargetType(baseName string) graphql.Output { +// findRelationTarget locates the GraphQL output type and its GVK for a relation target +func (re *RelationEnhancer) findRelationTarget(baseName string) (graphql.Output, *schema.GroupVersionKind, bool) { targetKind := cases.Title(language.English).String(baseName) for defKey, defSchema := range re.gateway.definitions { if re.matchesTargetKind(defSchema, targetKind) { + // Resolve or build the GraphQL type + var fieldType graphql.Output if existingType, exists := re.gateway.typesCache[defKey]; exists { - return existingType + fieldType = existingType + } else { + ft, _, err := re.gateway.convertSwaggerTypeToGraphQL(defSchema, defKey, []string{}, make(map[string]bool)) + if err != nil { + continue + } + fieldType = ft } - if fieldType, _, err := re.gateway.convertSwaggerTypeToGraphQL(defSchema, defKey, []string{}, make(map[string]bool)); err == nil { - return fieldType + // Extract GVK from the schema definition + gvk, err := re.gateway.getGroupVersionKind(defKey) + if err != nil || gvk == nil { + continue } + + return fieldType, gvk, true } } - return graphql.String + return nil, nil, false } // matchesTargetKind checks if a schema definition matches the target kind From 249f80aad0399818583c4514b1511b15a4d84f58 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 13:53:08 +0200 Subject: [PATCH 03/22] simplidied buildKindRegistry On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/resolver.go | 25 +++---- listener/pkg/apischema/builder.go | 114 ++++++++++++++++++------------ 2 files changed, 80 insertions(+), 59 deletions(-) diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index 11a2f079..04849f27 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -29,7 +29,6 @@ type Provider interface { CustomQueriesProvider CommonResolver() graphql.FieldResolveFn SanitizeGroupName(string) string - RuntimeClient() client.WithWatch RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn } @@ -94,15 +93,6 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) list.SetGroupVersionKind(schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind + "List"}) var opts []client.ListOption - if isResourceNamespaceScoped(scope) { - namespace, err := getStringArg(p.Args, NamespaceArg, false) - if err != nil { - return nil, err - } - if namespace != "" { - opts = append(opts, client.InNamespace(namespace)) - } - } if val, ok := p.Args[LabelSelectorArg].(string); ok && val != "" { selector, err := labels.Parse(val) @@ -113,6 +103,16 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) opts = append(opts, client.MatchingLabelsSelector{Selector: selector}) } + if isResourceNamespaceScoped(scope) { + namespace, err := getStringArg(p.Args, NamespaceArg, false) + if err != nil { + return nil, err + } + if namespace != "" { + opts = append(opts, client.InNamespace(namespace)) + } + } + if err = r.runtimeClient.List(ctx, list, opts...); err != nil { log.Error().Err(err).Str("scope", string(scope)).Msg("Unable to list objects") return nil, err @@ -463,11 +463,6 @@ func compareNumbers[T int64 | float64](a, b T) int { } } -// RuntimeClient returns the runtime client for use in relationship resolution -func (r *Service) RuntimeClient() client.WithWatch { - return r.runtimeClient -} - // RelationResolver creates a GraphQL resolver for relation fields func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { return r.relationResolver.CreateResolver(fieldName, gvk) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index ca9f0f37..133323af 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -254,7 +254,40 @@ func (b *SchemaBuilder) buildKindRegistry() { } // Index by lowercase kind name for consistent lookup - b.kindRegistry[strings.ToLower(gvk.Kind)] = append(b.kindRegistry[strings.ToLower(gvk.Kind)], resourceInfo) + key := strings.ToLower(gvk.Kind) + b.kindRegistry[key] = append(b.kindRegistry[key], resourceInfo) + } + + // Ensure deterministic order for picks: sort each slice by Group, Version, Kind, SchemaKey + for kindKey, infos := range b.kindRegistry { + slices.SortFunc(infos, func(a, b ResourceInfo) int { + if a.Group != b.Group { + if a.Group < b.Group { + return -1 + } + return 1 + } + if a.Version != b.Version { + if a.Version < b.Version { + return -1 + } + return 1 + } + if a.Kind != b.Kind { + if a.Kind < b.Kind { + return -1 + } + return 1 + } + if a.SchemaKey < b.SchemaKey { + return -1 + } + if a.SchemaKey > b.SchemaKey { + return 1 + } + return 0 + }) + b.kindRegistry[kindKey] = infos } b.log.Debug().Int("kindCount", len(b.kindRegistry)).Msg("built kind registry for relationships") @@ -266,51 +299,34 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { return } - // Create a copy of properties to avoid modifying while iterating - originalProps := make(map[string]spec.Schema) - for k, v := range schema.Properties { - originalProps[k] = v - } + for propName := range schema.Properties { + if !isRefProperty(propName) { + continue + } - for propName := range originalProps { - if strings.HasSuffix(propName, "Ref") && propName != "Ref" { - // Extract the base kind (e.g., "role" from "roleRef") - baseKind := strings.TrimSuffix(propName, "Ref") - b.log.Debug().Str("propName", propName).Str("baseKind", baseKind).Msg("Found Ref field") - - // Find the first matching resource type for this kind - lookupKey := strings.ToLower(baseKind) - b.log.Debug().Str("lookupKey", lookupKey).Msg("Looking up in kind registry") - if resourceTypes, exists := b.kindRegistry[lookupKey]; exists && len(resourceTypes) > 0 { - b.log.Debug().Str("lookupKey", lookupKey).Int("resourceCount", len(resourceTypes)).Msg("Found matching resources") - // Use the first matching resource type - targetResource := resourceTypes[0] - - // Generate field name (e.g., "role" for "roleRef") - fieldName := strings.ToLower(baseKind) - - // Add the relationship field - if _, exists := schema.Properties[fieldName]; !exists { - // Create a reference to the target schema - refSchema := spec.Schema{ - SchemaProps: spec.SchemaProps{ - Ref: spec.MustCreateRef(fmt.Sprintf("#/definitions/%s.%s.%s", - targetResource.Group, targetResource.Version, targetResource.Kind)), - }, - } - schema.Properties[fieldName] = refSchema - - b.log.Info(). - Str("sourceField", propName). - Str("targetField", fieldName). - Str("targetKind", targetResource.Kind). - Str("targetGroup", targetResource.Group). - Msg("Added relationship field") - } - } else { - b.log.Debug().Str("lookupKey", lookupKey).Msg("No matching resources found in kind registry") - } + baseKind := strings.TrimSuffix(propName, "Ref") + lookupKey := strings.ToLower(baseKind) + + resourceTypes, exists := b.kindRegistry[lookupKey] + if !exists || len(resourceTypes) == 0 { + continue + } + + fieldName := strings.ToLower(baseKind) + if _, exists := schema.Properties[fieldName]; exists { + continue } + + target := resourceTypes[0] + ref := spec.MustCreateRef(fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind)) + schema.Properties[fieldName] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}} + + b.log.Info(). + Str("sourceField", propName). + Str("targetField", fieldName). + Str("targetKind", target.Kind). + Str("targetGroup", target.Group). + Msg("Added relationship field") } // Recursively process nested objects and write back modifications @@ -322,6 +338,16 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { } } +func isRefProperty(name string) bool { + if !strings.HasSuffix(name, "Ref") { + return false + } + if name == "Ref" { + return false + } + return true +} + func (b *SchemaBuilder) Complete() ([]byte, error) { v3JSON, err := json.Marshal(&schemaResponse{ Components: schemasComponentsWrapper{ From 96a22a227d434dba5441d84b02401e201868f457 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 14:22:37 +0200 Subject: [PATCH 04/22] consistentcy On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index 5239cd4f..8baeda60 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -26,7 +26,7 @@ func NewRelationResolver(service *Service) *RelationResolver { // CreateResolver creates a GraphQL resolver for relation fields func (rr *RelationResolver) CreateResolver(fieldName string, targetGVK schema.GroupVersionKind) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { - parentObj, ok := p.Source.(map[string]interface{}) + parentObj, ok := p.Source.(map[string]any) if !ok { return nil, nil } @@ -49,7 +49,7 @@ type referenceInfo struct { } // extractReferenceInfo extracts reference details from a *Ref object -func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]interface{}, fieldName string) referenceInfo { +func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]any, fieldName string) referenceInfo { name, _ := parentObj["name"].(string) if name == "" { return referenceInfo{} From de3c7a54cf04f1274646937736ce5b4dcc6263dc Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 17:32:10 +0200 Subject: [PATCH 05/22] resolver On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 64 ++++++++++++++++++++--------------- gateway/resolver/resolver.go | 17 ++-------- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index 8baeda60..d5940073 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -6,50 +6,39 @@ import ( "github.com/graphql-go/graphql" "golang.org/x/text/cases" "golang.org/x/text/language" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" ) -// RelationResolver handles runtime resolution of relation fields -type RelationResolver struct { - service *Service -} - -// NewRelationResolver creates a new relation resolver -func NewRelationResolver(service *Service) *RelationResolver { - return &RelationResolver{ - service: service, - } +// referenceInfo holds extracted reference details +type referenceInfo struct { + name string + namespace string + kind string + apiGroup string } -// CreateResolver creates a GraphQL resolver for relation fields -func (rr *RelationResolver) CreateResolver(fieldName string, targetGVK schema.GroupVersionKind) graphql.FieldResolveFn { +// RelationResolver creates a GraphQL resolver for relation fields +func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { parentObj, ok := p.Source.(map[string]any) if !ok { return nil, nil } - refInfo := rr.extractReferenceInfo(parentObj, fieldName) + refInfo := r.extractReferenceInfo(parentObj, fieldName) if refInfo.name == "" { return nil, nil } - return rr.resolveReference(p.Context, refInfo, targetGVK) + return r.resolveReference(p.Context, refInfo, gvk) } } -// referenceInfo holds extracted reference details -type referenceInfo struct { - name string - namespace string - kind string - apiGroup string -} - // extractReferenceInfo extracts reference details from a *Ref object -func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]any, fieldName string) referenceInfo { +func (r *Service) extractReferenceInfo(parentObj map[string]any, fieldName string) referenceInfo { name, _ := parentObj["name"].(string) if name == "" { return referenceInfo{} @@ -73,7 +62,7 @@ func (rr *RelationResolver) extractReferenceInfo(parentObj map[string]any, field } // resolveReference fetches a referenced Kubernetes resource using provided target GVK -func (rr *RelationResolver) resolveReference(ctx context.Context, ref referenceInfo, targetGVK schema.GroupVersionKind) (interface{}, error) { +func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targetGVK schema.GroupVersionKind) (interface{}, error) { gvk := targetGVK // Allow overrides from the reference object if specified @@ -85,7 +74,7 @@ func (rr *RelationResolver) resolveReference(ctx context.Context, ref referenceI } // Convert sanitized group to original before calling the client - gvk.Group = rr.service.getOriginalGroupName(gvk.Group) + gvk.Group = r.getOriginalGroupName(gvk.Group) obj := &unstructured.Unstructured{} obj.SetGroupVersionKind(gvk) @@ -95,9 +84,28 @@ func (rr *RelationResolver) resolveReference(ctx context.Context, ref referenceI key.Namespace = ref.namespace } - if err := rr.service.runtimeClient.Get(ctx, key, obj); err == nil { - return obj.Object, nil + err := r.runtimeClient.Get(ctx, key, obj) + if err != nil { + // For "not found" errors, return nil to allow graceful degradation + // This handles cases where referenced resources are deleted or don't exist + if apierrors.IsNotFound(err) { + return nil, nil + } + + // For other errors (network, permission, etc.), log and return the actual error + // This ensures proper error propagation for debugging and monitoring + r.log.Error(). + Err(err). + Str("operation", "resolve_relation"). + Str("group", gvk.Group). + Str("version", gvk.Version). + Str("kind", gvk.Kind). + Str("name", ref.name). + Str("namespace", ref.namespace). + Msg("Unable to resolve referenced object") + return nil, err } - return nil, nil + // Happy path: resource found successfully + return obj.Object, nil } diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index 04849f27..e56798ab 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -50,22 +50,16 @@ type CustomQueriesProvider interface { type Service struct { log *logger.Logger // groupNames stores relation between sanitized group names and original group names that are used in the Kubernetes API - groupNames map[string]string // map[sanitizedGroupName]originalGroupName - runtimeClient client.WithWatch - relationResolver *RelationResolver + groupNames map[string]string // map[sanitizedGroupName]originalGroupName + runtimeClient client.WithWatch } func New(log *logger.Logger, runtimeClient client.WithWatch) *Service { - s := &Service{ + return &Service{ log: log, groupNames: make(map[string]string), runtimeClient: runtimeClient, } - - // Initialize the relation resolver - s.relationResolver = NewRelationResolver(s) - - return s } // ListItems returns a GraphQL CommonResolver function that lists Kubernetes resources of the given GroupVersionKind. @@ -462,8 +456,3 @@ func compareNumbers[T int64 | float64](a, b T) int { return 0 } } - -// RelationResolver creates a GraphQL resolver for relation fields -func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { - return r.relationResolver.CreateResolver(fieldName, gvk) -} From 0787ae7452cec914969c61fd97c2b792772f9a0a Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 17:39:53 +0200 Subject: [PATCH 06/22] removed circular dep from gateway On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/schema/relations.go | 50 ++++++++++++++----------------------- gateway/schema/schema.go | 6 +---- 2 files changed, 20 insertions(+), 36 deletions(-) diff --git a/gateway/schema/relations.go b/gateway/schema/relations.go index cd18d94d..e9dfa79d 100644 --- a/gateway/schema/relations.go +++ b/gateway/schema/relations.go @@ -11,20 +11,8 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -// RelationEnhancer handles schema enhancement for relation fields -type RelationEnhancer struct { - gateway *Gateway -} - -// NewRelationEnhancer creates a new relation enhancer -func NewRelationEnhancer(gateway *Gateway) *RelationEnhancer { - return &RelationEnhancer{ - gateway: gateway, - } -} - -// AddRelationFields adds relation fields to schemas that contain *Ref fields -func (re *RelationEnhancer) AddRelationFields(fields graphql.Fields, properties map[string]spec.Schema) { +// addRelationFields adds relation fields to schemas that contain *Ref fields +func (g *Gateway) addRelationFields(fields graphql.Fields, properties map[string]spec.Schema) { for fieldName := range properties { if !strings.HasSuffix(fieldName, "Ref") { continue @@ -38,7 +26,7 @@ func (re *RelationEnhancer) AddRelationFields(fields graphql.Fields, properties continue } - enhancedType := re.enhanceRefTypeWithRelation(refField.Type, baseName) + enhancedType := g.enhanceRefTypeWithRelation(refField.Type, baseName) if enhancedType == nil { continue } @@ -50,31 +38,31 @@ func (re *RelationEnhancer) AddRelationFields(fields graphql.Fields, properties } // enhanceRefTypeWithRelation adds a relation field to a *Ref object type -func (re *RelationEnhancer) enhanceRefTypeWithRelation(originalType graphql.Output, baseName string) graphql.Output { +func (g *Gateway) enhanceRefTypeWithRelation(originalType graphql.Output, baseName string) graphql.Output { objType, ok := originalType.(*graphql.Object) if !ok { return originalType } cacheKey := objType.Name() + "_" + baseName + "_Enhanced" - if enhancedType, exists := re.gateway.enhancedTypesCache[cacheKey]; exists { + if enhancedType, exists := g.enhancedTypesCache[cacheKey]; exists { return enhancedType } - enhancedFields := re.copyOriginalFields(objType.Fields()) - re.addRelationField(enhancedFields, baseName) + enhancedFields := g.copyOriginalFields(objType.Fields()) + g.addRelationField(enhancedFields, baseName) enhancedType := graphql.NewObject(graphql.ObjectConfig{ Name: sanitizeFieldName(cacheKey), Fields: enhancedFields, }) - re.gateway.enhancedTypesCache[cacheKey] = enhancedType + g.enhancedTypesCache[cacheKey] = enhancedType return enhancedType } // copyOriginalFields converts FieldDefinition to Field for reuse -func (re *RelationEnhancer) copyOriginalFields(originalFieldDefs graphql.FieldDefinitionMap) graphql.Fields { +func (g *Gateway) copyOriginalFields(originalFieldDefs graphql.FieldDefinitionMap) graphql.Fields { enhancedFields := make(graphql.Fields, len(originalFieldDefs)) for fieldName, fieldDef := range originalFieldDefs { enhancedFields[fieldName] = &graphql.Field{ @@ -87,8 +75,8 @@ func (re *RelationEnhancer) copyOriginalFields(originalFieldDefs graphql.FieldDe } // addRelationField adds a single relation field to the enhanced fields -func (re *RelationEnhancer) addRelationField(enhancedFields graphql.Fields, baseName string) { - targetType, targetGVK, ok := re.findRelationTarget(baseName) +func (g *Gateway) addRelationField(enhancedFields graphql.Fields, baseName string) { + targetType, targetGVK, ok := g.findRelationTarget(baseName) if !ok { return } @@ -96,22 +84,22 @@ func (re *RelationEnhancer) addRelationField(enhancedFields graphql.Fields, base sanitizedBaseName := sanitizeFieldName(baseName) enhancedFields[sanitizedBaseName] = &graphql.Field{ Type: targetType, - Resolve: re.gateway.resolver.RelationResolver(baseName, *targetGVK), + Resolve: g.resolver.RelationResolver(baseName, *targetGVK), } } // findRelationTarget locates the GraphQL output type and its GVK for a relation target -func (re *RelationEnhancer) findRelationTarget(baseName string) (graphql.Output, *schema.GroupVersionKind, bool) { +func (g *Gateway) findRelationTarget(baseName string) (graphql.Output, *schema.GroupVersionKind, bool) { targetKind := cases.Title(language.English).String(baseName) - for defKey, defSchema := range re.gateway.definitions { - if re.matchesTargetKind(defSchema, targetKind) { + for defKey, defSchema := range g.definitions { + if g.matchesTargetKind(defSchema, targetKind) { // Resolve or build the GraphQL type var fieldType graphql.Output - if existingType, exists := re.gateway.typesCache[defKey]; exists { + if existingType, exists := g.typesCache[defKey]; exists { fieldType = existingType } else { - ft, _, err := re.gateway.convertSwaggerTypeToGraphQL(defSchema, defKey, []string{}, make(map[string]bool)) + ft, _, err := g.convertSwaggerTypeToGraphQL(defSchema, defKey, []string{}, make(map[string]bool)) if err != nil { continue } @@ -119,7 +107,7 @@ func (re *RelationEnhancer) findRelationTarget(baseName string) (graphql.Output, } // Extract GVK from the schema definition - gvk, err := re.gateway.getGroupVersionKind(defKey) + gvk, err := g.getGroupVersionKind(defKey) if err != nil || gvk == nil { continue } @@ -132,7 +120,7 @@ func (re *RelationEnhancer) findRelationTarget(baseName string) (graphql.Output, } // matchesTargetKind checks if a schema definition matches the target kind -func (re *RelationEnhancer) matchesTargetKind(defSchema spec.Schema, targetKind string) bool { +func (g *Gateway) matchesTargetKind(defSchema spec.Schema, targetKind string) bool { gvkExt, ok := defSchema.Extensions["x-kubernetes-group-version-kind"] if !ok { return false diff --git a/gateway/schema/schema.go b/gateway/schema/schema.go index edaac1b7..8db274c7 100644 --- a/gateway/schema/schema.go +++ b/gateway/schema/schema.go @@ -29,7 +29,6 @@ type Gateway struct { typesCache map[string]*graphql.Object inputTypesCache map[string]*graphql.InputObject enhancedTypesCache map[string]*graphql.Object // Cache for enhanced *Ref types - relationEnhancer *RelationEnhancer // Prevents naming conflict in case of the same Kind name in different groups/versions typeNameRegistry map[string]string // map[Kind]GroupVersion @@ -49,9 +48,6 @@ func New(log *logger.Logger, definitions spec.Definitions, resolverProvider reso typeByCategory: make(map[string][]resolver.TypeByCategory), } - // Initialize the relation enhancer after gateway is created - g.relationEnhancer = NewRelationEnhancer(g) - err := g.generateGraphqlSchema() return g, err @@ -339,7 +335,7 @@ func (g *Gateway) generateGraphQLFields(resourceScheme *spec.Schema, typePrefix } // Add relation fields for any *Ref fields in this schema - g.relationEnhancer.AddRelationFields(fields, resourceScheme.Properties) + g.addRelationFields(fields, resourceScheme.Properties) return fields, inputFields, nil } From 44afd8ba15473960256817f40f0375069c92185e Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Fri, 15 Aug 2025 17:46:17 +0200 Subject: [PATCH 07/22] better sorting On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 133323af..59d51b39 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -261,31 +261,16 @@ func (b *SchemaBuilder) buildKindRegistry() { // Ensure deterministic order for picks: sort each slice by Group, Version, Kind, SchemaKey for kindKey, infos := range b.kindRegistry { slices.SortFunc(infos, func(a, b ResourceInfo) int { - if a.Group != b.Group { - if a.Group < b.Group { - return -1 - } - return 1 + if cmp := strings.Compare(a.Group, b.Group); cmp != 0 { + return cmp } - if a.Version != b.Version { - if a.Version < b.Version { - return -1 - } - return 1 + if cmp := strings.Compare(a.Version, b.Version); cmp != 0 { + return cmp } - if a.Kind != b.Kind { - if a.Kind < b.Kind { - return -1 - } - return 1 + if cmp := strings.Compare(a.Kind, b.Kind); cmp != 0 { + return cmp } - if a.SchemaKey < b.SchemaKey { - return -1 - } - if a.SchemaKey > b.SchemaKey { - return 1 - } - return 0 + return strings.Compare(a.SchemaKey, b.SchemaKey) }) b.kindRegistry[kindKey] = infos } From 9f144404f45a53944be985a2aa354aa85f2befda Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 11:54:44 +0200 Subject: [PATCH 08/22] smart sorting with preffered resource first On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 123 ++++++++++++++++++++++--- listener/pkg/apischema/crd_resolver.go | 2 + 2 files changed, 111 insertions(+), 14 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 59d51b39..1ed34fe7 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -34,10 +34,11 @@ var ( ) type SchemaBuilder struct { - schemas map[string]*spec.Schema - err *multierror.Error - log *logger.Logger - kindRegistry map[string][]ResourceInfo + schemas map[string]*spec.Schema + err *multierror.Error + log *logger.Logger + kindRegistry map[string][]ResourceInfo + preferredVersions map[string]string // map[group/kind]preferredVersion } // ResourceInfo holds information about a resource for relationship resolution @@ -50,9 +51,10 @@ type ResourceInfo struct { func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logger.Logger) *SchemaBuilder { b := &SchemaBuilder{ - schemas: make(map[string]*spec.Schema), - kindRegistry: make(map[string][]ResourceInfo), - log: log, + schemas: make(map[string]*spec.Schema), + kindRegistry: make(map[string][]ResourceInfo), + preferredVersions: make(map[string]string), + log: log, } apiv3Paths, err := oc.Paths() @@ -199,6 +201,33 @@ func (b *SchemaBuilder) WithApiResourceCategories(list []*metav1.APIResourceList return b } +// WithPreferredVersions populates preferred version information from API discovery +func (b *SchemaBuilder) WithPreferredVersions(apiResLists []*metav1.APIResourceList) *SchemaBuilder { + for _, apiResList := range apiResLists { + gv, err := runtimeSchema.ParseGroupVersion(apiResList.GroupVersion) + if err != nil { + b.log.Debug().Err(err).Str("groupVersion", apiResList.GroupVersion).Msg("failed to parse group version") + continue + } + + for _, resource := range apiResList.APIResources { + // Create a key for group/kind to track preferred version + key := fmt.Sprintf("%s/%s", gv.Group, resource.Kind) + + // Store this version as preferred for this group/kind + // ServerPreferredResources returns the preferred version for each group + b.preferredVersions[key] = gv.Version + + b.log.Debug(). + Str("group", gv.Group). + Str("kind", resource.Kind). + Str("preferredVersion", gv.Version). + Msg("registered preferred version") + } + } + return b +} + // WithRelationships adds relationship fields to schemas that have *Ref fields func (b *SchemaBuilder) WithRelationships() *SchemaBuilder { // Build kind registry first @@ -258,19 +287,35 @@ func (b *SchemaBuilder) buildKindRegistry() { b.kindRegistry[key] = append(b.kindRegistry[key], resourceInfo) } - // Ensure deterministic order for picks: sort each slice by Group, Version, Kind, SchemaKey + // Sort by preferred version first, then by stability and group priority for kindKey, infos := range b.kindRegistry { - slices.SortFunc(infos, func(a, b ResourceInfo) int { - if cmp := strings.Compare(a.Group, b.Group); cmp != 0 { - return cmp + slices.SortFunc(infos, func(a, bInfo ResourceInfo) int { + // 1. Prioritize resources with preferred versions + aKey := fmt.Sprintf("%s/%s", a.Group, a.Kind) + bKey := fmt.Sprintf("%s/%s", bInfo.Group, bInfo.Kind) + + aPreferred := b.preferredVersions[aKey] == a.Version + bPreferred := b.preferredVersions[bKey] == bInfo.Version + + if aPreferred && !bPreferred { + return -1 // a is preferred, comes first } - if cmp := strings.Compare(a.Version, b.Version); cmp != 0 { + if !aPreferred && bPreferred { + return 1 // b is preferred, comes first + } + + // 2. If both or neither are preferred, prioritize by group (core comes first) + if cmp := b.compareGroups(a.Group, bInfo.Group); cmp != 0 { return cmp } - if cmp := strings.Compare(a.Kind, b.Kind); cmp != 0 { + + // 3. Then by version stability (v1 > v1beta1 > v1alpha1) + if cmp := b.compareVersionStability(a.Version, bInfo.Version); cmp != 0 { return cmp } - return strings.Compare(a.SchemaKey, b.SchemaKey) + + // 4. Finally by schema key for deterministic ordering + return strings.Compare(a.SchemaKey, bInfo.SchemaKey) }) b.kindRegistry[kindKey] = infos } @@ -278,6 +323,56 @@ func (b *SchemaBuilder) buildKindRegistry() { b.log.Debug().Int("kindCount", len(b.kindRegistry)).Msg("built kind registry for relationships") } +// compareGroups prioritizes core Kubernetes groups over custom groups +func (b *SchemaBuilder) compareGroups(groupA, groupB string) int { + // Core group (empty string) comes first + if groupA == "" && groupB != "" { + return -1 + } + if groupA != "" && groupB == "" { + return 1 + } + + // k8s.io groups come before custom groups + aIsK8s := strings.Contains(groupA, "k8s.io") + bIsK8s := strings.Contains(groupB, "k8s.io") + + if aIsK8s && !bIsK8s { + return -1 + } + if !aIsK8s && bIsK8s { + return 1 + } + + // Otherwise alphabetical + return strings.Compare(groupA, groupB) +} + +// compareVersionStability prioritizes stable versions over beta/alpha +func (b *SchemaBuilder) compareVersionStability(versionA, versionB string) int { + aStability := b.getVersionStability(versionA) + bStability := b.getVersionStability(versionB) + + // Lower number = more stable (stable=0, beta=1, alpha=2) + if aStability != bStability { + return aStability - bStability + } + + // Same stability level, compare alphabetically + return strings.Compare(versionA, versionB) +} + +// getVersionStability returns stability priority (lower = more stable) +func (b *SchemaBuilder) getVersionStability(version string) int { + if strings.Contains(version, "alpha") { + return 2 // least stable + } + if strings.Contains(version, "beta") { + return 1 // somewhat stable + } + return 0 // most stable (v1, v2, etc.) +} + // expandRelationships detects fields ending with 'Ref' and adds corresponding relationship fields func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { if schema.Properties == nil { diff --git a/listener/pkg/apischema/crd_resolver.go b/listener/pkg/apischema/crd_resolver.go index a83d4469..b56be55d 100644 --- a/listener/pkg/apischema/crd_resolver.go +++ b/listener/pkg/apischema/crd_resolver.go @@ -76,6 +76,7 @@ func (cr *CRDResolver) ResolveApiSchema(crd *apiextensionsv1.CustomResourceDefin result, err := NewSchemaBuilder(cr.OpenAPIV3(), preferredApiGroups, cr.log). WithScope(cr.RESTMapper). + WithPreferredVersions(apiResLists). WithCRDCategories(crd). WithRelationships(). Complete() @@ -207,6 +208,7 @@ func (cr *CRDResolver) resolveSchema(dc discovery.DiscoveryInterface, rm meta.RE result, err := NewSchemaBuilder(dc.OpenAPIV3(), preferredApiGroups, cr.log). WithScope(rm). + WithPreferredVersions(apiResList). WithApiResourceCategories(apiResList). WithRelationships(). Complete() From 5cd1b848f38dd8547130b2e722c633ff8802117f Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 12:40:09 +0200 Subject: [PATCH 09/22] use GVK instead of Kind as a kay On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 103 ++++++++++++++++++------------ 1 file changed, 63 insertions(+), 40 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 1ed34fe7..47c26e9e 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -37,8 +37,8 @@ type SchemaBuilder struct { schemas map[string]*spec.Schema err *multierror.Error log *logger.Logger - kindRegistry map[string][]ResourceInfo - preferredVersions map[string]string // map[group/kind]preferredVersion + kindRegistry map[GroupVersionKind]ResourceInfo // Changed: Use GVK as key for precise lookup + preferredVersions map[string]string // map[group/kind]preferredVersion } // ResourceInfo holds information about a resource for relationship resolution @@ -52,7 +52,7 @@ type ResourceInfo struct { func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logger.Logger) *SchemaBuilder { b := &SchemaBuilder{ schemas: make(map[string]*spec.Schema), - kindRegistry: make(map[string][]ResourceInfo), + kindRegistry: make(map[GroupVersionKind]ResourceInfo), preferredVersions: make(map[string]string), log: log, } @@ -274,7 +274,7 @@ func (b *SchemaBuilder) buildKindRegistry() { gvk := gvks[0] - // Add to kind registry + // Add to kind registry with precise GVK key resourceInfo := ResourceInfo{ Group: gvk.Group, Version: gvk.Version, @@ -282,45 +282,70 @@ func (b *SchemaBuilder) buildKindRegistry() { SchemaKey: schemaKey, } - // Index by lowercase kind name for consistent lookup - key := strings.ToLower(gvk.Kind) - b.kindRegistry[key] = append(b.kindRegistry[key], resourceInfo) + // Index by full GroupVersionKind for precise lookup (no collisions) + gvkKey := GroupVersionKind{ + Group: gvk.Group, + Version: gvk.Version, + Kind: gvk.Kind, + } + b.kindRegistry[gvkKey] = resourceInfo } - // Sort by preferred version first, then by stability and group priority - for kindKey, infos := range b.kindRegistry { - slices.SortFunc(infos, func(a, bInfo ResourceInfo) int { - // 1. Prioritize resources with preferred versions - aKey := fmt.Sprintf("%s/%s", a.Group, a.Kind) - bKey := fmt.Sprintf("%s/%s", bInfo.Group, bInfo.Kind) - - aPreferred := b.preferredVersions[aKey] == a.Version - bPreferred := b.preferredVersions[bKey] == bInfo.Version + // No sorting needed - each GVK is now uniquely indexed + b.log.Debug().Int("gvkCount", len(b.kindRegistry)).Msg("built kind registry for relationships") +} - if aPreferred && !bPreferred { - return -1 // a is preferred, comes first - } - if !aPreferred && bPreferred { - return 1 // b is preferred, comes first - } +// findBestResourceForKind finds the best matching resource for a given kind name +// using preferred version logic and group prioritization +func (b *SchemaBuilder) findBestResourceForKind(kindName string) *ResourceInfo { + // Collect all resources with matching kind name + candidates := make([]ResourceInfo, 0) - // 2. If both or neither are preferred, prioritize by group (core comes first) - if cmp := b.compareGroups(a.Group, bInfo.Group); cmp != 0 { - return cmp - } + for gvk, resourceInfo := range b.kindRegistry { + if strings.EqualFold(gvk.Kind, kindName) { + candidates = append(candidates, resourceInfo) + } + } - // 3. Then by version stability (v1 > v1beta1 > v1alpha1) - if cmp := b.compareVersionStability(a.Version, bInfo.Version); cmp != 0 { - return cmp - } + if len(candidates) == 0 { + return nil + } - // 4. Finally by schema key for deterministic ordering - return strings.Compare(a.SchemaKey, bInfo.SchemaKey) - }) - b.kindRegistry[kindKey] = infos + if len(candidates) == 1 { + return &candidates[0] } - b.log.Debug().Int("kindCount", len(b.kindRegistry)).Msg("built kind registry for relationships") + // Sort candidates using preferred version logic + slices.SortFunc(candidates, func(a, bRes ResourceInfo) int { + // 1. Prioritize resources with preferred versions + aKey := fmt.Sprintf("%s/%s", a.Group, a.Kind) + bKey := fmt.Sprintf("%s/%s", bRes.Group, bRes.Kind) + + aPreferred := b.preferredVersions[aKey] == a.Version + bPreferred := b.preferredVersions[bKey] == bRes.Version + + if aPreferred && !bPreferred { + return -1 // a is preferred, comes first + } + if !aPreferred && bPreferred { + return 1 // b is preferred, comes first + } + + // 2. If both or neither are preferred, prioritize by group (core comes first) + if cmp := b.compareGroups(a.Group, bRes.Group); cmp != 0 { + return cmp + } + + // 3. Then by version stability (v1 > v1beta1 > v1alpha1) + if cmp := b.compareVersionStability(a.Version, bRes.Version); cmp != 0 { + return cmp + } + + // 4. Finally by schema key for deterministic ordering + return strings.Compare(a.SchemaKey, bRes.SchemaKey) + }) + + return &candidates[0] } // compareGroups prioritizes core Kubernetes groups over custom groups @@ -385,10 +410,10 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { } baseKind := strings.TrimSuffix(propName, "Ref") - lookupKey := strings.ToLower(baseKind) - resourceTypes, exists := b.kindRegistry[lookupKey] - if !exists || len(resourceTypes) == 0 { + // Find the best resource for this kind name using preferred version logic + target := b.findBestResourceForKind(baseKind) + if target == nil { continue } @@ -396,8 +421,6 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { if _, exists := schema.Properties[fieldName]; exists { continue } - - target := resourceTypes[0] ref := spec.MustCreateRef(fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind)) schema.Properties[fieldName] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}} From a9a360c6532f3e78a688ed9d5f8e72af22c50a5b Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 13:53:05 +0200 Subject: [PATCH 10/22] add test with preferred resource On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 43 +++++++++++++++++++ listener/pkg/apischema/relationships_test.go | 45 ++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 47c26e9e..59f07967 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -292,9 +292,52 @@ func (b *SchemaBuilder) buildKindRegistry() { } // No sorting needed - each GVK is now uniquely indexed + // Check for kinds with multiple resources but no preferred versions + b.warnAboutMissingPreferredVersions() + b.log.Debug().Int("gvkCount", len(b.kindRegistry)).Msg("built kind registry for relationships") } +// warnAboutMissingPreferredVersions checks for kinds with multiple resources but no preferred versions +func (b *SchemaBuilder) warnAboutMissingPreferredVersions() { + // Group resources by kind name to find potential conflicts + kindGroups := make(map[string][]ResourceInfo) + + for _, resourceInfo := range b.kindRegistry { + kindKey := strings.ToLower(resourceInfo.Kind) + kindGroups[kindKey] = append(kindGroups[kindKey], resourceInfo) + } + + // Check each kind that has multiple resources + for kindName, resources := range kindGroups { + if len(resources) <= 1 { + continue // No conflict possible + } + + // Check if any of the resources has a preferred version + hasPreferred := false + for _, resource := range resources { + key := fmt.Sprintf("%s/%s", resource.Group, resource.Kind) + if b.preferredVersions[key] == resource.Version { + hasPreferred = true + break + } + } + + // Warn if no preferred version found + if !hasPreferred { + groups := make([]string, 0, len(resources)) + for _, resource := range resources { + groups = append(groups, fmt.Sprintf("%s/%s", resource.Group, resource.Version)) + } + b.log.Warn(). + Str("kind", kindName). + Strs("availableResources", groups). + Msg("Multiple resources found for kind with no preferred version - using fallback resolution. Consider setting preferred versions for better API governance.") + } + } +} + // findBestResourceForKind finds the best matching resource for a given kind name // using preferred version logic and group prioritization func (b *SchemaBuilder) findBestResourceForKind(kindName string) *ResourceInfo { diff --git a/listener/pkg/apischema/relationships_test.go b/listener/pkg/apischema/relationships_test.go index d117508c..55fce67f 100644 --- a/listener/pkg/apischema/relationships_test.go +++ b/listener/pkg/apischema/relationships_test.go @@ -7,6 +7,7 @@ import ( apischema "github.com/openmfp/kubernetes-graphql-gateway/listener/pkg/apischema" apimocks "github.com/openmfp/kubernetes-graphql-gateway/listener/pkg/apischema/mocks" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/openapi" "k8s.io/kube-openapi/pkg/validation/spec" ) @@ -82,3 +83,47 @@ func Test_build_kind_registry_lowercases_keys_and_picks_first(t *testing.T) { // ensure it referenced the first group assert.Contains(t, added.Ref.String(), "#/definitions/a.example.v1.Thing") } + +func Test_preferred_version_takes_priority_over_fallback(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Multiple schemas with same Kind - a.example would win alphabetically, + // but we'll set z.last as preferred to verify it takes priority + childA := schemaWithGVK("a.example", "v1", "Child") + childB := schemaWithGVK("b.example", "v1", "Child") + childZ := schemaWithGVK("z.last", "v1", "Child") // would be last alphabetically + + b.SetSchemas(map[string]*spec.Schema{ + "a.example.v1.Child": childA, + "b.example.v1.Child": childB, + "z.last.v1.Child": childZ, + }) + + // Set z.last as preferred (even though it would be last alphabetically) + b.WithPreferredVersions([]*metav1.APIResourceList{ + { + GroupVersion: "z.last/v1", + APIResources: []metav1.APIResource{ + {Kind: "Child"}, + }, + }, + }) + + b.WithRelationships() + + // Add a parent schema that references childRef + parentSchema := &spec.Schema{SchemaProps: spec.SchemaProps{Properties: map[string]spec.Schema{ + "childRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + }}} + b.GetSchemas()["x.v1.Parent"] = parentSchema + + b.WithRelationships() + added, ok := b.GetSchemas()["x.v1.Parent"].Properties["child"] + assert.True(t, ok, "expected relationship field 'child'") + + // Should reference z.last because it's the preferred version, not a.example (alphabetical first) + assert.Contains(t, added.Ref.String(), "#/definitions/z.last.v1.Child", + "expected preferred version z.last to be chosen over alphabetically first a.example") +} From 54b8d924f64287be2feb48000a31a55a9da8f461 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 16:48:36 +0200 Subject: [PATCH 11/22] use map native methods On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 59f07967..7f57b013 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -125,15 +125,9 @@ func (b *SchemaBuilder) WithScope(rm meta.RESTMapper) *SchemaBuilder { } if namespaced { - if schema.VendorExtensible.Extensions == nil { - schema.VendorExtensible.Extensions = map[string]any{} - } - schema.VendorExtensible.Extensions[common.ScopeExtensionKey] = apiextensionsv1.NamespaceScoped + schema.VendorExtensible.AddExtension(common.ScopeExtensionKey, apiextensionsv1.NamespaceScoped) } else { - if schema.VendorExtensible.Extensions == nil { - schema.VendorExtensible.Extensions = map[string]any{} - } - schema.VendorExtensible.Extensions[common.ScopeExtensionKey] = apiextensionsv1.ClusterScoped + schema.VendorExtensible.AddExtension(common.ScopeExtensionKey, apiextensionsv1.ClusterScoped) } } return b @@ -161,10 +155,7 @@ func (b *SchemaBuilder) WithCRDCategories(crd *apiextensionsv1.CustomResourceDef b.log.Debug().Str("resource", resourceKey).Msg("no categories provided for CRD kind") continue } - if resourceSchema.VendorExtensible.Extensions == nil { - resourceSchema.VendorExtensible.Extensions = map[string]any{} - } - resourceSchema.VendorExtensible.Extensions[common.CategoriesExtensionKey] = crd.Spec.Names.Categories + resourceSchema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, crd.Spec.Names.Categories) b.schemas[resourceKey] = resourceSchema } return b @@ -191,10 +182,7 @@ func (b *SchemaBuilder) WithApiResourceCategories(list []*metav1.APIResourceList if !ok { continue } - if resourceSchema.VendorExtensible.Extensions == nil { - resourceSchema.VendorExtensible.Extensions = map[string]any{} - } - resourceSchema.VendorExtensible.Extensions[common.CategoriesExtensionKey] = apiResource.Categories + resourceSchema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, apiResource.Categories) b.schemas[resourceKey] = resourceSchema } } From f286765bbebb165b8e7de052456421290baf3b89 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 17:38:23 +0200 Subject: [PATCH 12/22] nesting On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 102 +++++++++++++++++++++++++----- 1 file changed, 86 insertions(+), 16 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 7f57b013..1d7b3a17 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -39,6 +39,8 @@ type SchemaBuilder struct { log *logger.Logger kindRegistry map[GroupVersionKind]ResourceInfo // Changed: Use GVK as key for precise lookup preferredVersions map[string]string // map[group/kind]preferredVersion + maxRelationDepth int // maximum allowed relationship nesting depth (1 = single level) + relationDepths map[string]int // tracks the minimum depth at which each schema is referenced } // ResourceInfo holds information about a resource for relationship resolution @@ -54,6 +56,8 @@ func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logge schemas: make(map[string]*spec.Schema), kindRegistry: make(map[GroupVersionKind]ResourceInfo), preferredVersions: make(map[string]string), + maxRelationDepth: 1, // Default to 1-level depth for now + relationDepths: make(map[string]int), log: log, } @@ -75,6 +79,19 @@ func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logge return b } +// WithMaxRelationDepth sets the maximum allowed relationship nesting depth +// depth=1: A->B (single level) +// depth=2: A->B->C (two levels) +// depth=3: A->B->C->D (three levels) +func (b *SchemaBuilder) WithMaxRelationDepth(depth int) *SchemaBuilder { + if depth < 1 { + depth = 1 // Minimum depth is 1 + } + b.maxRelationDepth = depth + b.log.Info().Int("maxRelationDepth", depth).Msg("Set maximum relationship nesting depth") + return b +} + type GroupVersionKind struct { Group string `json:"group"` Version string `json:"version"` @@ -221,16 +238,64 @@ func (b *SchemaBuilder) WithRelationships() *SchemaBuilder { // Build kind registry first b.buildKindRegistry() - // Expand relationships in all schemas - b.log.Info().Int("kindRegistrySize", len(b.kindRegistry)).Msg("Starting relationship expansion") - for schemaKey, schema := range b.schemas { - b.log.Debug().Str("schemaKey", schemaKey).Msg("Processing schema for relationships") - b.expandRelationships(schema) + // For depth=1: use simple relation target tracking (working approach) + // For depth>1: use iterative expansion (scalable approach) + if b.maxRelationDepth == 1 { + b.expandWithSimpleDepthControl() + } else { + b.expandWithConfigurableDepthControl() } return b } +// expandWithSimpleDepthControl implements the working 1-level depth control +func (b *SchemaBuilder) expandWithSimpleDepthControl() { + // First pass: identify relation targets + relationTargets := make(map[string]bool) + for _, schema := range b.schemas { + if schema.Properties == nil { + continue + } + for propName := range schema.Properties { + if !isRefProperty(propName) { + continue + } + baseKind := strings.TrimSuffix(propName, "Ref") + target := b.findBestResourceForKind(baseKind) + if target != nil { + relationTargets[target.SchemaKey] = true + } + } + } + + b.log.Info(). + Int("kindRegistrySize", len(b.kindRegistry)). + Int("relationTargets", len(relationTargets)). + Msg("Starting 1-level relationship expansion") + + // Second pass: expand only non-targets + for schemaKey, schema := range b.schemas { + if relationTargets[schemaKey] { + b.log.Debug().Str("schemaKey", schemaKey).Msg("Skipping relation target (1-level depth control)") + continue + } + b.expandRelationshipsSimple(schema, schemaKey) + } +} + +// expandWithConfigurableDepthControl implements scalable depth control for depth > 1 +func (b *SchemaBuilder) expandWithConfigurableDepthControl() { + b.log.Info(). + Int("kindRegistrySize", len(b.kindRegistry)). + Int("maxRelationDepth", b.maxRelationDepth). + Msg("Starting configurable relationship expansion") + + // TODO: Implement proper multi-level depth control + // For now, fall back to simple approach + b.expandWithSimpleDepthControl() +} + // buildKindRegistry builds a map of kind names to available resource types func (b *SchemaBuilder) buildKindRegistry() { for schemaKey, schema := range b.schemas { @@ -286,6 +351,9 @@ func (b *SchemaBuilder) buildKindRegistry() { b.log.Debug().Int("gvkCount", len(b.kindRegistry)).Msg("built kind registry for relationships") } +// TODO: Implement proper multi-level depth calculation when needed +// For now, focusing on the working 1-level depth control + // warnAboutMissingPreferredVersions checks for kinds with multiple resources but no preferred versions func (b *SchemaBuilder) warnAboutMissingPreferredVersions() { // Group resources by kind name to find potential conflicts @@ -429,8 +497,8 @@ func (b *SchemaBuilder) getVersionStability(version string) int { return 0 // most stable (v1, v2, etc.) } -// expandRelationships detects fields ending with 'Ref' and adds corresponding relationship fields -func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { +// expandRelationshipsSimple adds relationship fields for the simple 1-level depth control +func (b *SchemaBuilder) expandRelationshipsSimple(schema *spec.Schema, schemaKey string) { if schema.Properties == nil { return } @@ -452,7 +520,15 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { if _, exists := schema.Properties[fieldName]; exists { continue } - ref := spec.MustCreateRef(fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind)) + + // Create proper reference - handle empty group (core) properly + var refPath string + if target.Group == "" { + refPath = fmt.Sprintf("#/definitions/%s.%s", target.Version, target.Kind) + } else { + refPath = fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind) + } + ref := spec.MustCreateRef(refPath) schema.Properties[fieldName] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}} b.log.Info(). @@ -460,16 +536,10 @@ func (b *SchemaBuilder) expandRelationships(schema *spec.Schema) { Str("targetField", fieldName). Str("targetKind", target.Kind). Str("targetGroup", target.Group). + Str("refPath", refPath). + Str("sourceSchema", schemaKey). Msg("Added relationship field") } - - // Recursively process nested objects and write back modifications - for key, prop := range schema.Properties { - if prop.Type.Contains("object") && prop.Properties != nil { - b.expandRelationships(&prop) - schema.Properties[key] = prop - } - } } func isRefProperty(name string) bool { From c2bca5b832f6e25ed09257d8861a6d56d9189b0f Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Mon, 18 Aug 2025 18:19:55 +0200 Subject: [PATCH 13/22] limit relation resolver by getItem only On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 112 +++++++++++++++++++++++++++++++ gateway/resolver/resolver.go | 14 ++++ gateway/resolver/subscription.go | 11 +++ 3 files changed, 137 insertions(+) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index d5940073..b5e96c37 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -2,8 +2,10 @@ package resolver import ( "context" + "strings" "github.com/graphql-go/graphql" + "go.opentelemetry.io/otel/trace" "golang.org/x/text/cases" "golang.org/x/text/language" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -21,8 +23,30 @@ type referenceInfo struct { } // RelationResolver creates a GraphQL resolver for relation fields +// Relationships are only enabled for GetItem queries to prevent N+1 problems in ListItems and Subscriptions func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { + // Try context first, fallback to GraphQL info analysis + operation := r.getOperationFromContext(p.Context) + if operation == "unknown" { + operation = r.detectOperationFromGraphQLInfo(p) + } + + r.log.Debug(). + Str("fieldName", fieldName). + Str("operation", operation). + Str("graphqlField", p.Info.FieldName). + Msg("RelationResolver called") + + // Check if relationships are allowed in this query context + if !r.isRelationResolutionAllowedForOperation(operation) { + r.log.Debug(). + Str("fieldName", fieldName). + Str("operation", operation). + Msg("Relationship resolution disabled for this operation type") + return nil, nil + } + parentObj, ok := p.Source.(map[string]any) if !ok { return nil, nil @@ -109,3 +133,91 @@ func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targe // Happy path: resource found successfully return obj.Object, nil } + +// isRelationResolutionAllowed checks if relationship resolution should be enabled for this operation +// Only allows relationships in GetItem operations to prevent N+1 problems +func (r *Service) isRelationResolutionAllowed(ctx context.Context) bool { + operation := r.getOperationFromContext(ctx) + return r.isRelationResolutionAllowedForOperation(operation) +} + +// isRelationResolutionAllowedForOperation checks if relationship resolution should be enabled for the given operation type +func (r *Service) isRelationResolutionAllowedForOperation(operation string) bool { + // Only allow relationships for GetItem and GetItemAsYAML operations + switch operation { + case "GetItem", "GetItemAsYAML": + return true + case "ListItems", "SubscribeItem", "SubscribeItems": + return false + default: + // For unknown operations, be conservative and disable relationships + r.log.Debug().Str("operation", operation).Msg("Unknown operation type, disabling relationships") + return false + } +} + +// Context key for tracking operation type +type operationContextKey string + +const OperationTypeKey operationContextKey = "operation_type" + +// getOperationFromContext extracts the operation name from the context +func (r *Service) getOperationFromContext(ctx context.Context) string { + // Try to get operation from context value first + if op, ok := ctx.Value(OperationTypeKey).(string); ok { + return op + } + + // Fallback: try to extract from trace span name + span := trace.SpanFromContext(ctx) + if span == nil { + return "unknown" + } + + // This is a workaround - we'll need to get the span name somehow + // For now, assume unknown and rely on context values + return "unknown" +} + +// detectOperationFromGraphQLInfo analyzes GraphQL field path to determine operation type +// This looks at the parent field context to determine if we're in a list, single item, or subscription +func (r *Service) detectOperationFromGraphQLInfo(p graphql.ResolveParams) string { + if p.Info.Path == nil { + return "unknown" + } + + // Walk up the path to find the parent resolver context + path := p.Info.Path + for path.Prev != nil { + path = path.Prev + + // Check if we find a parent field that indicates the operation type + if fieldName, ok := path.Key.(string); ok { + fieldLower := strings.ToLower(fieldName) + + // Check for subscription patterns + if strings.Contains(fieldLower, "subscription") { + r.log.Debug(). + Str("parentField", fieldName). + Msg("Detected subscription context from parent field") + return "SubscribeItems" + } + + // Check for list patterns (plural without args, or explicitly plural fields) + if strings.HasSuffix(fieldName, "s") && !strings.HasSuffix(fieldName, "Status") { + // This looks like a plural field, likely a list operation + r.log.Debug(). + Str("parentField", fieldName). + Msg("Detected list context from parent field") + return "ListItems" + } + } + } + + // If we can't determine from parent context, assume it's a single item operation + // This is the safe default that allows relationships + r.log.Debug(). + Str("currentField", p.Info.FieldName). + Msg("Could not determine operation from path, defaulting to GetItem") + return "GetItem" +} diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index e56798ab..9dc8484b 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -2,6 +2,7 @@ package resolver import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -68,6 +69,11 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) ctx, span := otel.Tracer("").Start(p.Context, "ListItems", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() + // Add operation type to context to disable relationship resolution + ctx = context.WithValue(ctx, operationContextKey("operation_type"), "ListItems") + // Update p.Context so field resolvers inherit the operation type + p.Context = ctx + gvk.Group = r.getOriginalGroupName(gvk.Group) log, err := r.log.ChildLoggerWithAttributes( @@ -142,6 +148,11 @@ func (r *Service) GetItem(gvk schema.GroupVersionKind, scope v1.ResourceScope) g ctx, span := otel.Tracer("").Start(p.Context, "GetItem", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() + // Add operation type to context to enable relationship resolution + ctx = context.WithValue(ctx, operationContextKey("operation_type"), "GetItem") + // Update p.Context so field resolvers inherit the operation type + p.Context = ctx + gvk.Group = r.getOriginalGroupName(gvk.Group) log, err := r.log.ChildLoggerWithAttributes( @@ -195,6 +206,9 @@ func (r *Service) GetItemAsYAML(gvk schema.GroupVersionKind, scope v1.ResourceSc p.Context, span = otel.Tracer("").Start(p.Context, "GetItemAsYAML", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() + // Add operation type to context to enable relationship resolution + p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "GetItemAsYAML") + out, err := r.GetItem(gvk, scope)(p) if err != nil { return "", err diff --git a/gateway/resolver/subscription.go b/gateway/resolver/subscription.go index 3b0216d4..8601938c 100644 --- a/gateway/resolver/subscription.go +++ b/gateway/resolver/subscription.go @@ -1,6 +1,7 @@ package resolver import ( + "context" "fmt" "reflect" "sort" @@ -32,6 +33,10 @@ func (r *Service) SubscribeItem(gvk schema.GroupVersionKind, scope v1.ResourceSc return func(p graphql.ResolveParams) (interface{}, error) { _, span := otel.Tracer("").Start(p.Context, "SubscribeItem", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() + + // Add operation type to context to disable relationship resolution + p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "SubscribeItem") + resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, true, scope) @@ -43,6 +48,10 @@ func (r *Service) SubscribeItems(gvk schema.GroupVersionKind, scope v1.ResourceS return func(p graphql.ResolveParams) (interface{}, error) { _, span := otel.Tracer("").Start(p.Context, "SubscribeItems", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() + + // Add operation type to context to disable relationship resolution + p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "SubscribeItems") + resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, false, scope) @@ -353,6 +362,8 @@ func CreateSubscriptionResolver(isSingle bool) graphql.FieldResolveFn { return nil, err } + // Note: The context already contains operation type from SubscribeItem/SubscribeItems + // This will propagate to relationship resolvers, disabling them for subscriptions return source, nil } } From e578071c82f125a7574a3eecfda43e0a19e1ef93 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Tue, 19 Aug 2025 13:00:10 +0200 Subject: [PATCH 14/22] fix conflict On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 31 +- listener/pkg/apischema/builder.go | 266 +++++++++------- listener/pkg/apischema/relationships_test.go | 306 ++++++++++++++++++- 3 files changed, 449 insertions(+), 154 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index b5e96c37..44bc13b2 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -85,31 +85,29 @@ func (r *Service) extractReferenceInfo(parentObj map[string]any, fieldName strin } } -// resolveReference fetches a referenced Kubernetes resource using provided target GVK +// resolveReference fetches a referenced Kubernetes resource using strict conflict resolution func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targetGVK schema.GroupVersionKind) (interface{}, error) { - gvk := targetGVK - - // Allow overrides from the reference object if specified + // Use provided reference info to override GVK if specified + finalGVK := targetGVK if ref.apiGroup != "" { - gvk.Group = ref.apiGroup + finalGVK.Group = ref.apiGroup } if ref.kind != "" { - gvk.Kind = ref.kind + finalGVK.Kind = ref.kind } // Convert sanitized group to original before calling the client - gvk.Group = r.getOriginalGroupName(gvk.Group) + finalGVK.Group = r.getOriginalGroupName(finalGVK.Group) obj := &unstructured.Unstructured{} - obj.SetGroupVersionKind(gvk) + obj.SetGroupVersionKind(finalGVK) key := client.ObjectKey{Name: ref.name} if ref.namespace != "" { key.Namespace = ref.namespace } - err := r.runtimeClient.Get(ctx, key, obj) - if err != nil { + if err := r.runtimeClient.Get(ctx, key, obj); err != nil { // For "not found" errors, return nil to allow graceful degradation // This handles cases where referenced resources are deleted or don't exist if apierrors.IsNotFound(err) { @@ -121,9 +119,9 @@ func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targe r.log.Error(). Err(err). Str("operation", "resolve_relation"). - Str("group", gvk.Group). - Str("version", gvk.Version). - Str("kind", gvk.Kind). + Str("group", finalGVK.Group). + Str("version", finalGVK.Version). + Str("kind", finalGVK.Kind). Str("name", ref.name). Str("namespace", ref.namespace). Msg("Unable to resolve referenced object") @@ -134,13 +132,6 @@ func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targe return obj.Object, nil } -// isRelationResolutionAllowed checks if relationship resolution should be enabled for this operation -// Only allows relationships in GetItem operations to prevent N+1 problems -func (r *Service) isRelationResolutionAllowed(ctx context.Context) bool { - operation := r.getOperationFromContext(ctx) - return r.isRelationResolutionAllowedForOperation(operation) -} - // isRelationResolutionAllowedForOperation checks if relationship resolution should be enabled for the given operation type func (r *Service) isRelationResolutionAllowedForOperation(operation string) bool { // Only allow relationships for GetItem and GetItemAsYAML operations diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 1d7b3a17..59f75a73 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -262,9 +262,11 @@ func (b *SchemaBuilder) expandWithSimpleDepthControl() { continue } baseKind := strings.TrimSuffix(propName, "Ref") - target := b.findBestResourceForKind(baseKind) - if target != nil { - relationTargets[target.SchemaKey] = true + candidates := b.findAllCandidatesForKind(baseKind) + + // Mark all candidates as relation targets + for _, candidate := range candidates { + relationTargets[candidate.SchemaKey] = true } } } @@ -342,6 +344,14 @@ func (b *SchemaBuilder) buildKindRegistry() { Kind: gvk.Kind, } b.kindRegistry[gvkKey] = resourceInfo + + // DEBUG: Log each resource added to kind registry + b.log.Info(). + Str("group", gvk.Group). + Str("version", gvk.Version). + Str("kind", gvk.Kind). + Str("schemaKey", schemaKey). + Msg("DEBUG: Added resource to kind registry") } // No sorting needed - each GVK is now uniquely indexed @@ -394,152 +404,172 @@ func (b *SchemaBuilder) warnAboutMissingPreferredVersions() { } } -// findBestResourceForKind finds the best matching resource for a given kind name -// using preferred version logic and group prioritization -func (b *SchemaBuilder) findBestResourceForKind(kindName string) *ResourceInfo { - // Collect all resources with matching kind name - candidates := make([]ResourceInfo, 0) - - for gvk, resourceInfo := range b.kindRegistry { - if strings.EqualFold(gvk.Kind, kindName) { - candidates = append(candidates, resourceInfo) - } - } - - if len(candidates) == 0 { - return nil - } - - if len(candidates) == 1 { - return &candidates[0] +// expandRelationshipsSimple adds relationship fields for the simple 1-level depth control +func (b *SchemaBuilder) expandRelationshipsSimple(schema *spec.Schema, schemaKey string) { + if schema.Properties == nil { + return } - // Sort candidates using preferred version logic - slices.SortFunc(candidates, func(a, bRes ResourceInfo) int { - // 1. Prioritize resources with preferred versions - aKey := fmt.Sprintf("%s/%s", a.Group, a.Kind) - bKey := fmt.Sprintf("%s/%s", bRes.Group, bRes.Kind) - - aPreferred := b.preferredVersions[aKey] == a.Version - bPreferred := b.preferredVersions[bKey] == bRes.Version - - if aPreferred && !bPreferred { - return -1 // a is preferred, comes first - } - if !aPreferred && bPreferred { - return 1 // b is preferred, comes first - } - - // 2. If both or neither are preferred, prioritize by group (core comes first) - if cmp := b.compareGroups(a.Group, bRes.Group); cmp != 0 { - return cmp - } - - // 3. Then by version stability (v1 > v1beta1 > v1alpha1) - if cmp := b.compareVersionStability(a.Version, bRes.Version); cmp != 0 { - return cmp + for propName := range schema.Properties { + if !isRefProperty(propName) { + continue } - // 4. Finally by schema key for deterministic ordering - return strings.Compare(a.SchemaKey, bRes.SchemaKey) - }) + baseKind := strings.TrimSuffix(propName, "Ref") - return &candidates[0] + // Check for conflicts and potentially add relationship field + b.processReferenceField(schema, schemaKey, propName, baseKind) + } } -// compareGroups prioritizes core Kubernetes groups over custom groups -func (b *SchemaBuilder) compareGroups(groupA, groupB string) int { - // Core group (empty string) comes first - if groupA == "" && groupB != "" { - return -1 - } - if groupA != "" && groupB == "" { - return 1 - } +// processReferenceField handles individual reference field processing with conflict detection +func (b *SchemaBuilder) processReferenceField(schema *spec.Schema, schemaKey, propName, baseKind string) { + // Find all candidates for this kind + candidates := b.findAllCandidatesForKind(baseKind) - // k8s.io groups come before custom groups - aIsK8s := strings.Contains(groupA, "k8s.io") - bIsK8s := strings.Contains(groupB, "k8s.io") + // DEBUG: Log what candidates we found + b.log.Info(). + Str("baseKind", baseKind). + Str("sourceField", propName). + Str("sourceSchema", schemaKey). + Int("candidateCount", len(candidates)). + Msg("DEBUG: Processing reference field") + + switch len(candidates) { + case 0: + // No candidates found - skip relationship field generation + b.log.Debug(). + Str("kind", baseKind). + Str("sourceField", propName). + Str("sourceSchema", schemaKey). + Msg("No candidates found for kind - skipping relationship field") + return - if aIsK8s && !bIsK8s { - return -1 - } - if !aIsK8s && bIsK8s { - return 1 - } + case 1: + // Single candidate - generate relationship field normally + target := &candidates[0] + b.addRelationshipField(schema, schemaKey, propName, baseKind, target) - // Otherwise alphabetical - return strings.Compare(groupA, groupB) + default: + // Multiple candidates - enforce disambiguation in the *Ref field itself + b.enforceDisambiguationInRefField(schema, schemaKey, propName, baseKind, candidates) + } } -// compareVersionStability prioritizes stable versions over beta/alpha -func (b *SchemaBuilder) compareVersionStability(versionA, versionB string) int { - aStability := b.getVersionStability(versionA) - bStability := b.getVersionStability(versionB) +// findAllCandidatesForKind finds all resources that match the given kind name +func (b *SchemaBuilder) findAllCandidatesForKind(kindName string) []ResourceInfo { + candidates := make([]ResourceInfo, 0) - // Lower number = more stable (stable=0, beta=1, alpha=2) - if aStability != bStability { - return aStability - bStability + for gvk, resourceInfo := range b.kindRegistry { + if strings.EqualFold(gvk.Kind, kindName) { + candidates = append(candidates, resourceInfo) + } } - // Same stability level, compare alphabetically - return strings.Compare(versionA, versionB) + return candidates } -// getVersionStability returns stability priority (lower = more stable) -func (b *SchemaBuilder) getVersionStability(version string) int { - if strings.Contains(version, "alpha") { - return 2 // least stable +// addRelationshipField adds a relationship field for unambiguous references +func (b *SchemaBuilder) addRelationshipField(schema *spec.Schema, schemaKey, propName, baseKind string, target *ResourceInfo) { + fieldName := strings.ToLower(baseKind) + if _, exists := schema.Properties[fieldName]; exists { + return } - if strings.Contains(version, "beta") { - return 1 // somewhat stable + + // Create proper reference - handle empty group (core) properly + var refPath string + if target.Group == "" { + refPath = fmt.Sprintf("#/definitions/%s.%s", target.Version, target.Kind) + } else { + refPath = fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind) } - return 0 // most stable (v1, v2, etc.) + ref := spec.MustCreateRef(refPath) + schema.Properties[fieldName] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}} + + b.log.Info(). + Str("sourceField", propName). + Str("targetField", fieldName). + Str("targetKind", target.Kind). + Str("targetGroup", target.Group). + Str("refPath", refPath). + Str("sourceSchema", schemaKey). + Msg("Added relationship field") } -// expandRelationshipsSimple adds relationship fields for the simple 1-level depth control -func (b *SchemaBuilder) expandRelationshipsSimple(schema *spec.Schema, schemaKey string) { - if schema.Properties == nil { +// enforceDisambiguationInRefField modifies the *Ref field schema to require disambiguation +func (b *SchemaBuilder) enforceDisambiguationInRefField(schema *spec.Schema, schemaKey, propName, baseKind string, candidates []ResourceInfo) { + // Check if the *Ref field exists + if _, exists := schema.Properties[propName]; !exists { return } - for propName := range schema.Properties { - if !isRefProperty(propName) { - continue - } + // Modify the *Ref field to require apiGroup and kind for disambiguation + refFieldSchema := &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"string"}, + Description: "Name of the referenced resource", + }, + }, + "namespace": { + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"string"}, + Description: "Namespace of the referenced resource (if namespaced)", + }, + }, + "apiGroup": { + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"string"}, + Description: fmt.Sprintf("API group of the referenced %s (required due to multiple groups providing this kind)", baseKind), + }, + }, + "kind": { + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"string"}, + Description: fmt.Sprintf("Kind of the referenced resource (required due to multiple groups providing %s)", baseKind), + }, + }, + }, + Required: []string{"name", "apiGroup", "kind"}, // Enforce disambiguation + Description: fmt.Sprintf("Reference to %s resource. Multiple API groups provide this kind: %s. "+ + "apiGroup and kind fields are required for disambiguation.", + baseKind, b.formatCandidateGroups(candidates)), + }, + } - baseKind := strings.TrimSuffix(propName, "Ref") + // Update the schema + schema.Properties[propName] = *refFieldSchema - // Find the best resource for this kind name using preferred version logic - target := b.findBestResourceForKind(baseKind) - if target == nil { - continue - } + // Log the enforcement + groups := b.formatCandidateGroups(candidates) + b.log.Warn(). + Str("kind", baseKind). + Str("sourceField", propName). + Str("sourceSchema", schemaKey). + Strs("conflictingGroups", groups). + Msg("SCHEMA ENFORCEMENT: Modified *Ref field to require apiGroup/kind due to conflicts") - fieldName := strings.ToLower(baseKind) - if _, exists := schema.Properties[fieldName]; exists { - continue - } + // Do NOT add automatic relationship field - user must be explicit + b.log.Info(). + Str("kind", baseKind). + Str("sourceField", propName). + Msg("Skipped automatic relationship field generation - user must specify explicit references") +} - // Create proper reference - handle empty group (core) properly - var refPath string - if target.Group == "" { - refPath = fmt.Sprintf("#/definitions/%s.%s", target.Version, target.Kind) +// formatCandidateGroups formats candidate groups for error messages +func (b *SchemaBuilder) formatCandidateGroups(candidates []ResourceInfo) []string { + groups := make([]string, len(candidates)) + for i, candidate := range candidates { + if candidate.Group == "" { + groups[i] = fmt.Sprintf("core/%s", candidate.Version) } else { - refPath = fmt.Sprintf("#/definitions/%s.%s.%s", target.Group, target.Version, target.Kind) + groups[i] = fmt.Sprintf("%s/%s", candidate.Group, candidate.Version) } - ref := spec.MustCreateRef(refPath) - schema.Properties[fieldName] = spec.Schema{SchemaProps: spec.SchemaProps{Ref: ref}} - - b.log.Info(). - Str("sourceField", propName). - Str("targetField", fieldName). - Str("targetKind", target.Kind). - Str("targetGroup", target.Group). - Str("refPath", refPath). - Str("sourceSchema", schemaKey). - Msg("Added relationship field") } + return groups } func isRefProperty(name string) bool { diff --git a/listener/pkg/apischema/relationships_test.go b/listener/pkg/apischema/relationships_test.go index 55fce67f..e9ec2aa4 100644 --- a/listener/pkg/apischema/relationships_test.go +++ b/listener/pkg/apischema/relationships_test.go @@ -54,12 +54,12 @@ func Test_with_relationships_adds_single_target_field(t *testing.T) { assert.Contains(t, added.Ref.String(), "#/definitions/g.v1.Role") } -func Test_build_kind_registry_lowercases_keys_and_picks_first(t *testing.T) { +func Test_schema_enforcement_prevents_conflicting_relationship_fields(t *testing.T) { mock := apimocks.NewMockClient(t) mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) - // Two schemas with same Kind different groups/versions - first should win + // Two schemas with same Kind different groups - should trigger schema enforcement first := schemaWithGVK("a.example", "v1", "Thing") second := schemaWithGVK("b.example", "v1", "Thing") b.SetSchemas(map[string]*spec.Schema{ @@ -70,27 +70,31 @@ func Test_build_kind_registry_lowercases_keys_and_picks_first(t *testing.T) { b.WithRelationships() // indirectly builds the registry - // validate lowercase key exists and contains both, but expansion uses first (covered by previous test) - // we assert the registry was built by triggering another schema that references thingRef + // Add a schema that references thingRef - should trigger conflict enforcement sRef := &spec.Schema{SchemaProps: spec.SchemaProps{Properties: map[string]spec.Schema{ "thingRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, }}} b.GetSchemas()["x.v1.HasThing"] = sRef b.WithRelationships() - added, ok := b.GetSchemas()["x.v1.HasThing"].Properties["thing"] - assert.True(t, ok, "expected relationship field 'thing'") - // ensure it referenced the first group - assert.Contains(t, added.Ref.String(), "#/definitions/a.example.v1.Thing") + + // Schema enforcement should PREVENT automatic relationship field generation due to conflicts + _, hasAutoField := b.GetSchemas()["x.v1.HasThing"].Properties["thing"] + assert.False(t, hasAutoField, "automatic relationship field should NOT be generated due to conflicts") + + // Schema enforcement should modify the *Ref field to require disambiguation + thingRefField := b.GetSchemas()["x.v1.HasThing"].Properties["thingRef"] + assert.Contains(t, thingRefField.Required, "apiGroup", "apiGroup should be required due to conflicts") + assert.Contains(t, thingRefField.Required, "kind", "kind should be required due to conflicts") + assert.Contains(t, thingRefField.Description, "Multiple API groups", "description should mention conflicts") } -func Test_preferred_version_takes_priority_over_fallback(t *testing.T) { +func Test_schema_enforcement_with_preferred_versions_still_requires_disambiguation(t *testing.T) { mock := apimocks.NewMockClient(t) mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) - // Multiple schemas with same Kind - a.example would win alphabetically, - // but we'll set z.last as preferred to verify it takes priority + // Multiple schemas with same Kind - conflicts exist even with preferred versions childA := schemaWithGVK("a.example", "v1", "Child") childB := schemaWithGVK("b.example", "v1", "Child") childZ := schemaWithGVK("z.last", "v1", "Child") // would be last alphabetically @@ -120,10 +124,280 @@ func Test_preferred_version_takes_priority_over_fallback(t *testing.T) { b.GetSchemas()["x.v1.Parent"] = parentSchema b.WithRelationships() - added, ok := b.GetSchemas()["x.v1.Parent"].Properties["child"] - assert.True(t, ok, "expected relationship field 'child'") - // Should reference z.last because it's the preferred version, not a.example (alphabetical first) - assert.Contains(t, added.Ref.String(), "#/definitions/z.last.v1.Child", - "expected preferred version z.last to be chosen over alphabetically first a.example") + // Even with preferred versions, strict mode should prevent automatic field generation with conflicts + _, hasAutoField := b.GetSchemas()["x.v1.Parent"].Properties["child"] + assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even with preferred versions when conflicts exist") + + // Schema enforcement should modify the *Ref field to require disambiguation + childRefField := b.GetSchemas()["x.v1.Parent"].Properties["childRef"] + assert.Contains(t, childRefField.Required, "apiGroup", "apiGroup should be required due to conflicts") + assert.Contains(t, childRefField.Required, "kind", "kind should be required due to conflicts") +} + +func Test_depth_control_prevents_deep_nesting(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Create a chain: Root -> Pod -> Service + // Only Root should get relationship fields, Pod and Service should be marked as targets + rootSchema := schemaWithGVK("example.com", "v1", "Root") + rootSchema.Properties = map[string]spec.Schema{ + "podRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + podSchema := schemaWithGVK("", "v1", "Pod") // Core group + podSchema.Properties = map[string]spec.Schema{ + "serviceRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + serviceSchema := schemaWithGVK("", "v1", "Service") // Core group + + b.SetSchemas(map[string]*spec.Schema{ + "example.com.v1.Root": rootSchema, + ".v1.Pod": podSchema, + ".v1.Service": serviceSchema, + }) + + // Verify default depth is 1 + b.WithRelationships() + + schemas := b.GetSchemas() + + // Root should get 'pod' relationship field (depth 0 -> 1) + _, hasPodField := schemas["example.com.v1.Root"].Properties["pod"] + assert.True(t, hasPodField, "Root should get pod relationship field") + + // Pod should NOT get 'service' relationship field (would be depth 1 -> 2, exceeds limit) + _, hasServiceField := schemas[".v1.Pod"].Properties["service"] + assert.False(t, hasServiceField, "Pod should NOT get service relationship field due to depth limit") + + // Service should not have any relationship fields added + originalServiceProps := len(serviceSchema.Properties) + currentServiceProps := len(schemas[".v1.Service"].Properties) + assert.Equal(t, originalServiceProps, currentServiceProps, "Service should not have relationship fields added") +} + +func Test_configurable_depth_control_api(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Test that WithMaxRelationDepth API works + b.WithMaxRelationDepth(2) + + // Create simple schemas to verify the builder accepts the API + rootSchema := schemaWithGVK("example.com", "v1", "Root") + b.SetSchemas(map[string]*spec.Schema{ + "example.com.v1.Root": rootSchema, + }) + + // Should not panic or error + b.WithRelationships() + + // For now, even with depth=2, it falls back to depth=1 behavior (as per TODO) + // This test verifies the API works and doesn't break anything +} + +func Test_single_level_prevents_circular_relationships(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Create circular reference: A -> B, B -> A + aSchema := schemaWithGVK("example.com", "v1", "A") + aSchema.Properties = map[string]spec.Schema{ + "bRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + bSchema := schemaWithGVK("example.com", "v1", "B") + bSchema.Properties = map[string]spec.Schema{ + "aRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + b.SetSchemas(map[string]*spec.Schema{ + "example.com.v1.A": aSchema, + "example.com.v1.B": bSchema, + }) + + b.WithRelationships() + + schemas := b.GetSchemas() + + // With 1-level depth control, both A and B are marked as relation targets + // So neither should get automatic relationship fields + _, hasAField := schemas["example.com.v1.A"].Properties["b"] + _, hasBField := schemas["example.com.v1.B"].Properties["a"] + + // At least one should not have the field to prevent infinite circular expansion + circularPrevented := !hasAField || !hasBField + assert.True(t, circularPrevented, "Circular relationships should be prevented by depth control") +} + +func Test_depth_control_with_multiple_chains(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Multiple chains: Chain1 (Root1 -> Pod), Chain2 (Root2 -> Service) + root1Schema := schemaWithGVK("example.com", "v1", "Root1") + root1Schema.Properties = map[string]spec.Schema{ + "podRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + root2Schema := schemaWithGVK("example.com", "v1", "Root2") + root2Schema.Properties = map[string]spec.Schema{ + "serviceRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + podSchema := schemaWithGVK("", "v1", "Pod") + serviceSchema := schemaWithGVK("", "v1", "Service") + + b.SetSchemas(map[string]*spec.Schema{ + "example.com.v1.Root1": root1Schema, + "example.com.v1.Root2": root2Schema, + ".v1.Pod": podSchema, + ".v1.Service": serviceSchema, + }) + + b.WithRelationships() + + schemas := b.GetSchemas() + + // Both roots should be able to reference their targets (no conflicts between chains) + _, hasPodField := schemas["example.com.v1.Root1"].Properties["pod"] + _, hasServiceField := schemas["example.com.v1.Root2"].Properties["service"] + + assert.True(t, hasPodField, "Root1 should get pod relationship field") + assert.True(t, hasServiceField, "Root2 should get service relationship field") + + // Targets (Pod, Service) should not get any additional relationship fields + assert.Empty(t, schemas[".v1.Pod"].Properties, "Pod should not have relationship fields (is a target)") + assert.Empty(t, schemas[".v1.Service"].Properties, "Service should not have relationship fields (is a target)") +} + +func Test_same_kind_different_groups_with_explicit_disambiguation(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Create two different groups providing the same "Database" kind + mysqlDB := schemaWithGVK("mysql.example.com", "v1", "Database") + postgresDB := schemaWithGVK("postgres.example.com", "v1", "Database") + + // Parent schema that wants to reference one of the databases + appSchema := schemaWithGVK("apps.example.com", "v1", "Application") + appSchema.Properties = map[string]spec.Schema{ + "databaseRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + b.SetSchemas(map[string]*spec.Schema{ + "mysql.example.com.v1.Database": mysqlDB, + "postgres.example.com.v1.Database": postgresDB, + "apps.example.com.v1.Application": appSchema, + }) + + b.WithRelationships() + + schemas := b.GetSchemas() + + // Verify schema enforcement was applied + _, hasAutoField := schemas["apps.example.com.v1.Application"].Properties["database"] + assert.False(t, hasAutoField, "automatic relationship field should NOT be generated due to kind conflicts") + + // Verify the databaseRef field was modified to require disambiguation + dbRefField := schemas["apps.example.com.v1.Application"].Properties["databaseRef"] + assert.NotEmpty(t, dbRefField.Required, "databaseRef should have required fields") + assert.Contains(t, dbRefField.Required, "apiGroup", "apiGroup should be required to choose between mysql.example.com and postgres.example.com") + assert.Contains(t, dbRefField.Required, "kind", "kind should be required for disambiguation") + + // Verify the description explains the conflict + assert.Contains(t, dbRefField.Description, "mysql.example.com", "description should mention mysql group") + assert.Contains(t, dbRefField.Description, "postgres.example.com", "description should mention postgres group") + assert.Contains(t, dbRefField.Description, "Multiple API groups", "description should explain the conflict") +} + +func Test_same_kind_different_groups_kubernetes_core_vs_custom(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Simulate core Kubernetes Service vs custom Service + coreService := schemaWithGVK("", "v1", "Service") // Core group (empty) + customService := schemaWithGVK("custom.example.com", "v1", "Service") + + // Parent that references "Service" - which one? + parentSchema := schemaWithGVK("example.com", "v1", "Parent") + parentSchema.Properties = map[string]spec.Schema{ + "serviceRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + + b.SetSchemas(map[string]*spec.Schema{ + ".v1.Service": coreService, + "custom.example.com.v1.Service": customService, + "example.com.v1.Parent": parentSchema, + }) + + b.WithRelationships() + + schemas := b.GetSchemas() + + // Even with core vs custom, should still require disambiguation + _, hasAutoField := schemas["example.com.v1.Parent"].Properties["service"] + assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even for core vs custom conflicts") + + // Schema should enforce disambiguation + serviceRefField := schemas["example.com.v1.Parent"].Properties["serviceRef"] + assert.Contains(t, serviceRefField.Required, "apiGroup", "apiGroup should be required to distinguish core vs custom Service") + assert.Contains(t, serviceRefField.Required, "kind", "kind should be required") + + // Description should mention both core and custom groups + description := serviceRefField.Description + assert.Contains(t, description, "core/v1", "description should mention core group") + assert.Contains(t, description, "custom.example.com/v1", "description should mention custom group") +} + +func Test_same_kind_different_groups_with_preferred_version_still_conflicts(t *testing.T) { + mock := apimocks.NewMockClient(t) + mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) + b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) + + // Multiple "Storage" providers with preferred version set + s3Storage := schemaWithGVK("aws.example.com", "v1", "Storage") + gcsStorage := schemaWithGVK("gcp.example.com", "v1", "Storage") + azureStorage := schemaWithGVK("azure.example.com", "v1", "Storage") + + b.SetSchemas(map[string]*spec.Schema{ + "aws.example.com.v1.Storage": s3Storage, + "gcp.example.com.v1.Storage": gcsStorage, + "azure.example.com.v1.Storage": azureStorage, + }) + + // Set preferred version for one of them + b.WithPreferredVersions([]*metav1.APIResourceList{ + { + GroupVersion: "aws.example.com/v1", + APIResources: []metav1.APIResource{{Kind: "Storage"}}, + }, + }) + + // Parent that wants to reference storage + appSchema := schemaWithGVK("apps.example.com", "v1", "BackupApp") + appSchema.Properties = map[string]spec.Schema{ + "storageRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, + } + b.GetSchemas()["apps.example.com.v1.BackupApp"] = appSchema + + b.WithRelationships() + + schemas := b.GetSchemas() + + // Even with preferred version, strict mode should still prevent automatic generation + _, hasAutoField := schemas["apps.example.com.v1.BackupApp"].Properties["storage"] + assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even with preferred version when multiple groups provide the same kind") + + // Should still require explicit disambiguation + storageRefField := schemas["apps.example.com.v1.BackupApp"].Properties["storageRef"] + assert.Contains(t, storageRefField.Required, "apiGroup", "apiGroup should be required even with preferred version to avoid ambiguity") } From b5d9b0be0749d7f241b7eef42e51e5e04eaa8dd4 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 09:09:01 +0200 Subject: [PATCH 15/22] reproduced kubeconfig conflict resolution logic On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 177 +++++++++---------- listener/pkg/apischema/relationships_test.go | 66 +++---- 2 files changed, 113 insertions(+), 130 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 59f75a73..1c1b2bab 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -345,13 +345,6 @@ func (b *SchemaBuilder) buildKindRegistry() { } b.kindRegistry[gvkKey] = resourceInfo - // DEBUG: Log each resource added to kind registry - b.log.Info(). - Str("group", gvk.Group). - Str("version", gvk.Version). - Str("kind", gvk.Kind). - Str("schemaKey", schemaKey). - Msg("DEBUG: Added resource to kind registry") } // No sorting needed - each GVK is now uniquely indexed @@ -417,26 +410,17 @@ func (b *SchemaBuilder) expandRelationshipsSimple(schema *spec.Schema, schemaKey baseKind := strings.TrimSuffix(propName, "Ref") - // Check for conflicts and potentially add relationship field + // Add relationship field using kubectl-style priority resolution b.processReferenceField(schema, schemaKey, propName, baseKind) } } -// processReferenceField handles individual reference field processing with conflict detection +// processReferenceField handles individual reference field processing with kubectl-style priority resolution func (b *SchemaBuilder) processReferenceField(schema *spec.Schema, schemaKey, propName, baseKind string) { - // Find all candidates for this kind - candidates := b.findAllCandidatesForKind(baseKind) + // Find best resource using kubectl-style priority + bestResource := b.findBestResourceForKind(baseKind) - // DEBUG: Log what candidates we found - b.log.Info(). - Str("baseKind", baseKind). - Str("sourceField", propName). - Str("sourceSchema", schemaKey). - Int("candidateCount", len(candidates)). - Msg("DEBUG: Processing reference field") - - switch len(candidates) { - case 0: + if bestResource == nil { // No candidates found - skip relationship field generation b.log.Debug(). Str("kind", baseKind). @@ -444,16 +428,36 @@ func (b *SchemaBuilder) processReferenceField(schema *spec.Schema, schemaKey, pr Str("sourceSchema", schemaKey). Msg("No candidates found for kind - skipping relationship field") return + } + + // Generate relationship field using the best resource + b.addRelationshipField(schema, schemaKey, propName, baseKind, bestResource) +} + +// findBestResourceForKind finds the best resource for a kind using kubectl-style priority resolution +func (b *SchemaBuilder) findBestResourceForKind(kindName string) *ResourceInfo { + candidates := b.findAllCandidatesForKind(kindName) - case 1: - // Single candidate - generate relationship field normally - target := &candidates[0] - b.addRelationshipField(schema, schemaKey, propName, baseKind, target) + if len(candidates) == 0 { + return nil + } - default: - // Multiple candidates - enforce disambiguation in the *Ref field itself - b.enforceDisambiguationInRefField(schema, schemaKey, propName, baseKind, candidates) + if len(candidates) == 1 { + return &candidates[0] } + + // Multiple candidates - use kubectl-style priority resolution + best := b.selectByKubectlPriority(candidates) + + // Log warning about the conflict for observability + groups := b.formatCandidateGroups(candidates) + b.log.Warn(). + Str("kind", kindName). + Str("selectedGroup", b.formatGroupVersion(best)). + Strs("availableGroups", groups). + Msg("Multiple API groups provide this kind - selected first by priority (kubectl-style)") + + return &best } // findAllCandidatesForKind finds all resources that match the given kind name @@ -469,6 +473,60 @@ func (b *SchemaBuilder) findAllCandidatesForKind(kindName string) []ResourceInfo return candidates } +// selectByKubectlPriority selects the best resource using kubectl's priority rules +func (sb *SchemaBuilder) selectByKubectlPriority(candidates []ResourceInfo) ResourceInfo { + // Sort candidates by kubectl priority: + // 1. Preferred versions first + // 2. Core groups (empty group) over extensions + // 3. Alphabetical by group name + // 4. Alphabetical by version (newer versions typically sort later) + slices.SortFunc(candidates, func(a, b ResourceInfo) int { + // 1. Check preferred versions first + aPreferred := sb.isPreferredVersion(a) + bPreferred := sb.isPreferredVersion(b) + if aPreferred && !bPreferred { + return -1 // a wins + } + if !aPreferred && bPreferred { + return 1 // b wins + } + + // 2. Core groups (empty group) beat extension groups + aCoreGroup := (a.Group == "") + bCoreGroup := (b.Group == "") + if aCoreGroup && !bCoreGroup { + return -1 // a wins (core group) + } + if !aCoreGroup && bCoreGroup { + return 1 // b wins (core group) + } + + // 3. Alphabetical by group name + if cmp := strings.Compare(a.Group, b.Group); cmp != 0 { + return cmp + } + + // 4. Alphabetical by version (this gives deterministic results) + return strings.Compare(a.Version, b.Version) + }) + + return candidates[0] // Return the first (highest priority) candidate +} + +// isPreferredVersion checks if this resource version is marked as preferred +func (b *SchemaBuilder) isPreferredVersion(resource ResourceInfo) bool { + key := fmt.Sprintf("%s/%s", resource.Group, resource.Kind) + return b.preferredVersions[key] == resource.Version +} + +// formatGroupVersion formats a resource for display +func (b *SchemaBuilder) formatGroupVersion(resource ResourceInfo) string { + if resource.Group == "" { + return fmt.Sprintf("core/%s", resource.Version) + } + return fmt.Sprintf("%s/%s", resource.Group, resource.Version) +} + // addRelationshipField adds a relationship field for unambiguous references func (b *SchemaBuilder) addRelationshipField(schema *spec.Schema, schemaKey, propName, baseKind string, target *ResourceInfo) { fieldName := strings.ToLower(baseKind) @@ -496,69 +554,6 @@ func (b *SchemaBuilder) addRelationshipField(schema *spec.Schema, schemaKey, pro Msg("Added relationship field") } -// enforceDisambiguationInRefField modifies the *Ref field schema to require disambiguation -func (b *SchemaBuilder) enforceDisambiguationInRefField(schema *spec.Schema, schemaKey, propName, baseKind string, candidates []ResourceInfo) { - // Check if the *Ref field exists - if _, exists := schema.Properties[propName]; !exists { - return - } - - // Modify the *Ref field to require apiGroup and kind for disambiguation - refFieldSchema := &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: spec.StringOrArray{"object"}, - Properties: map[string]spec.Schema{ - "name": { - SchemaProps: spec.SchemaProps{ - Type: spec.StringOrArray{"string"}, - Description: "Name of the referenced resource", - }, - }, - "namespace": { - SchemaProps: spec.SchemaProps{ - Type: spec.StringOrArray{"string"}, - Description: "Namespace of the referenced resource (if namespaced)", - }, - }, - "apiGroup": { - SchemaProps: spec.SchemaProps{ - Type: spec.StringOrArray{"string"}, - Description: fmt.Sprintf("API group of the referenced %s (required due to multiple groups providing this kind)", baseKind), - }, - }, - "kind": { - SchemaProps: spec.SchemaProps{ - Type: spec.StringOrArray{"string"}, - Description: fmt.Sprintf("Kind of the referenced resource (required due to multiple groups providing %s)", baseKind), - }, - }, - }, - Required: []string{"name", "apiGroup", "kind"}, // Enforce disambiguation - Description: fmt.Sprintf("Reference to %s resource. Multiple API groups provide this kind: %s. "+ - "apiGroup and kind fields are required for disambiguation.", - baseKind, b.formatCandidateGroups(candidates)), - }, - } - - // Update the schema - schema.Properties[propName] = *refFieldSchema - - // Log the enforcement - groups := b.formatCandidateGroups(candidates) - b.log.Warn(). - Str("kind", baseKind). - Str("sourceField", propName). - Str("sourceSchema", schemaKey). - Strs("conflictingGroups", groups). - Msg("SCHEMA ENFORCEMENT: Modified *Ref field to require apiGroup/kind due to conflicts") - - // Do NOT add automatic relationship field - user must be explicit - b.log.Info(). - Str("kind", baseKind). - Str("sourceField", propName). - Msg("Skipped automatic relationship field generation - user must specify explicit references") -} - // formatCandidateGroups formats candidate groups for error messages func (b *SchemaBuilder) formatCandidateGroups(candidates []ResourceInfo) []string { groups := make([]string, len(candidates)) diff --git a/listener/pkg/apischema/relationships_test.go b/listener/pkg/apischema/relationships_test.go index e9ec2aa4..0521d8b0 100644 --- a/listener/pkg/apischema/relationships_test.go +++ b/listener/pkg/apischema/relationships_test.go @@ -54,12 +54,12 @@ func Test_with_relationships_adds_single_target_field(t *testing.T) { assert.Contains(t, added.Ref.String(), "#/definitions/g.v1.Role") } -func Test_schema_enforcement_prevents_conflicting_relationship_fields(t *testing.T) { +func Test_kubectl_style_priority_resolution_for_conflicts(t *testing.T) { mock := apimocks.NewMockClient(t) mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) - // Two schemas with same Kind different groups - should trigger schema enforcement + // Two schemas with same Kind different groups - should use kubectl-style priority first := schemaWithGVK("a.example", "v1", "Thing") second := schemaWithGVK("b.example", "v1", "Thing") b.SetSchemas(map[string]*spec.Schema{ @@ -70,7 +70,7 @@ func Test_schema_enforcement_prevents_conflicting_relationship_fields(t *testing b.WithRelationships() // indirectly builds the registry - // Add a schema that references thingRef - should trigger conflict enforcement + // Add a schema that references thingRef - should use priority resolution sRef := &spec.Schema{SchemaProps: spec.SchemaProps{Properties: map[string]spec.Schema{ "thingRef": {SchemaProps: spec.SchemaProps{Type: spec.StringOrArray{"object"}}}, }}} @@ -78,18 +78,17 @@ func Test_schema_enforcement_prevents_conflicting_relationship_fields(t *testing b.WithRelationships() - // Schema enforcement should PREVENT automatic relationship field generation due to conflicts + // Kubectl-style resolution should GENERATE automatic relationship field using priority _, hasAutoField := b.GetSchemas()["x.v1.HasThing"].Properties["thing"] - assert.False(t, hasAutoField, "automatic relationship field should NOT be generated due to conflicts") + assert.True(t, hasAutoField, "automatic relationship field should be generated using kubectl-style priority") - // Schema enforcement should modify the *Ref field to require disambiguation + // The *Ref field should remain unchanged (backward compatible) thingRefField := b.GetSchemas()["x.v1.HasThing"].Properties["thingRef"] - assert.Contains(t, thingRefField.Required, "apiGroup", "apiGroup should be required due to conflicts") - assert.Contains(t, thingRefField.Required, "kind", "kind should be required due to conflicts") - assert.Contains(t, thingRefField.Description, "Multiple API groups", "description should mention conflicts") + assert.NotContains(t, thingRefField.Required, "apiGroup", "apiGroup should NOT be required - backward compatible") + assert.NotContains(t, thingRefField.Required, "kind", "kind should NOT be required - backward compatible") } -func Test_schema_enforcement_with_preferred_versions_still_requires_disambiguation(t *testing.T) { +func Test_kubectl_style_priority_respects_preferred_versions(t *testing.T) { mock := apimocks.NewMockClient(t) mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) @@ -125,14 +124,14 @@ func Test_schema_enforcement_with_preferred_versions_still_requires_disambiguati b.WithRelationships() - // Even with preferred versions, strict mode should prevent automatic field generation with conflicts + // Kubectl-style resolution should use preferred version to generate relationship field _, hasAutoField := b.GetSchemas()["x.v1.Parent"].Properties["child"] - assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even with preferred versions when conflicts exist") + assert.True(t, hasAutoField, "automatic relationship field should be generated using preferred version priority") - // Schema enforcement should modify the *Ref field to require disambiguation + // The *Ref field should remain unchanged (backward compatible) childRefField := b.GetSchemas()["x.v1.Parent"].Properties["childRef"] - assert.Contains(t, childRefField.Required, "apiGroup", "apiGroup should be required due to conflicts") - assert.Contains(t, childRefField.Required, "kind", "kind should be required due to conflicts") + assert.NotContains(t, childRefField.Required, "apiGroup", "apiGroup should NOT be required - backward compatible") + assert.NotContains(t, childRefField.Required, "kind", "kind should NOT be required - backward compatible") } func Test_depth_control_prevents_deep_nesting(t *testing.T) { @@ -302,20 +301,14 @@ func Test_same_kind_different_groups_with_explicit_disambiguation(t *testing.T) schemas := b.GetSchemas() - // Verify schema enforcement was applied + // Verify kubectl-style resolution was applied _, hasAutoField := schemas["apps.example.com.v1.Application"].Properties["database"] - assert.False(t, hasAutoField, "automatic relationship field should NOT be generated due to kind conflicts") + assert.True(t, hasAutoField, "automatic relationship field should be generated using kubectl-style priority") - // Verify the databaseRef field was modified to require disambiguation + // Verify the databaseRef field remains unchanged (backward compatible) dbRefField := schemas["apps.example.com.v1.Application"].Properties["databaseRef"] - assert.NotEmpty(t, dbRefField.Required, "databaseRef should have required fields") - assert.Contains(t, dbRefField.Required, "apiGroup", "apiGroup should be required to choose between mysql.example.com and postgres.example.com") - assert.Contains(t, dbRefField.Required, "kind", "kind should be required for disambiguation") - - // Verify the description explains the conflict - assert.Contains(t, dbRefField.Description, "mysql.example.com", "description should mention mysql group") - assert.Contains(t, dbRefField.Description, "postgres.example.com", "description should mention postgres group") - assert.Contains(t, dbRefField.Description, "Multiple API groups", "description should explain the conflict") + assert.NotContains(t, dbRefField.Required, "apiGroup", "apiGroup should NOT be required - backward compatible") + assert.NotContains(t, dbRefField.Required, "kind", "kind should NOT be required - backward compatible") } func Test_same_kind_different_groups_kubernetes_core_vs_custom(t *testing.T) { @@ -345,17 +338,12 @@ func Test_same_kind_different_groups_kubernetes_core_vs_custom(t *testing.T) { // Even with core vs custom, should still require disambiguation _, hasAutoField := schemas["example.com.v1.Parent"].Properties["service"] - assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even for core vs custom conflicts") + assert.True(t, hasAutoField, "automatic relationship field should be generated using kubectl-style priority (core wins)") - // Schema should enforce disambiguation + // The serviceRef field should remain unchanged (backward compatible) serviceRefField := schemas["example.com.v1.Parent"].Properties["serviceRef"] - assert.Contains(t, serviceRefField.Required, "apiGroup", "apiGroup should be required to distinguish core vs custom Service") - assert.Contains(t, serviceRefField.Required, "kind", "kind should be required") - - // Description should mention both core and custom groups - description := serviceRefField.Description - assert.Contains(t, description, "core/v1", "description should mention core group") - assert.Contains(t, description, "custom.example.com/v1", "description should mention custom group") + assert.NotContains(t, serviceRefField.Required, "apiGroup", "apiGroup should NOT be required - backward compatible") + assert.NotContains(t, serviceRefField.Required, "kind", "kind should NOT be required - backward compatible") } func Test_same_kind_different_groups_with_preferred_version_still_conflicts(t *testing.T) { @@ -393,11 +381,11 @@ func Test_same_kind_different_groups_with_preferred_version_still_conflicts(t *t schemas := b.GetSchemas() - // Even with preferred version, strict mode should still prevent automatic generation + // Kubectl-style resolution should use preferred version and generate relationship field _, hasAutoField := schemas["apps.example.com.v1.BackupApp"].Properties["storage"] - assert.False(t, hasAutoField, "automatic relationship field should NOT be generated even with preferred version when multiple groups provide the same kind") + assert.True(t, hasAutoField, "automatic relationship field should be generated using preferred version priority") - // Should still require explicit disambiguation + // The storageRef field should remain unchanged (backward compatible) storageRefField := schemas["apps.example.com.v1.BackupApp"].Properties["storageRef"] - assert.Contains(t, storageRefField.Required, "apiGroup", "apiGroup should be required even with preferred version to avoid ambiguity") + assert.NotContains(t, storageRefField.Required, "apiGroup", "apiGroup should NOT be required - backward compatible") } From 2ece5faaaccdd97f4739089170af9b12483f68d9 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 10:23:29 +0200 Subject: [PATCH 16/22] fix tests On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/reconciler/kcp/virtual_workspace_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/listener/reconciler/kcp/virtual_workspace_test.go b/listener/reconciler/kcp/virtual_workspace_test.go index 03373ce3..80a2808b 100644 --- a/listener/reconciler/kcp/virtual_workspace_test.go +++ b/listener/reconciler/kcp/virtual_workspace_test.go @@ -663,9 +663,9 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { Name: "test-ws", URL: "https://example.com", }, - expectError: true, // Expected due to kubeconfig dependency in metadata injection - expectedWriteCalls: 0, // Won't reach write due to metadata injection failure - errorShouldContain: "failed to inject KCP cluster metadata", + expectError: false, // Metadata injection uses default kubeconfig location + expectedWriteCalls: 1, // Should successfully write the schema + errorShouldContain: "", }, { name: "io_write_error", @@ -674,9 +674,9 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { URL: "https://example.com", }, ioWriteError: errors.New("write failed"), - expectError: true, // Expected due to kubeconfig dependency in metadata injection - expectedWriteCalls: 0, // Won't reach write due to metadata injection failure - errorShouldContain: "failed to inject KCP cluster metadata", + expectError: true, // Expected due to IO write failure + expectedWriteCalls: 1, // Will attempt to write but fail + errorShouldContain: "failed to write schema file", }, { name: "api_resolve_error", From e5b5c8f63c4bc46db3bc26042ffd91199d6c1d00 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 11:00:30 +0200 Subject: [PATCH 17/22] fix tests 2 On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- .../reconciler/kcp/virtual_workspace_test.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/listener/reconciler/kcp/virtual_workspace_test.go b/listener/reconciler/kcp/virtual_workspace_test.go index 80a2808b..8af7a80f 100644 --- a/listener/reconciler/kcp/virtual_workspace_test.go +++ b/listener/reconciler/kcp/virtual_workspace_test.go @@ -663,9 +663,9 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { Name: "test-ws", URL: "https://example.com", }, - expectError: false, // Metadata injection uses default kubeconfig location - expectedWriteCalls: 1, // Should successfully write the schema - errorShouldContain: "", + expectError: true, // Expected due to kubeconfig dependency in metadata injection + expectedWriteCalls: 0, // Won't reach write due to metadata injection failure in CI + errorShouldContain: "failed to inject KCP cluster metadata", }, { name: "io_write_error", @@ -674,9 +674,9 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { URL: "https://example.com", }, ioWriteError: errors.New("write failed"), - expectError: true, // Expected due to IO write failure - expectedWriteCalls: 1, // Will attempt to write but fail - errorShouldContain: "failed to write schema file", + expectError: true, // Expected due to kubeconfig dependency in metadata injection + expectedWriteCalls: 0, // Won't reach write due to metadata injection failure in CI + errorShouldContain: "failed to inject KCP cluster metadata", }, { name: "api_resolve_error", @@ -695,8 +695,13 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Set up test environment where KUBECONFIG is not available oldKubeconfig := os.Getenv("KUBECONFIG") - defer os.Setenv("KUBECONFIG", oldKubeconfig) + oldHome := os.Getenv("HOME") + defer func() { + os.Setenv("KUBECONFIG", oldKubeconfig) + os.Setenv("HOME", oldHome) + }() os.Unsetenv("KUBECONFIG") + os.Setenv("HOME", "/nonexistent") // Force metadata injection to fail consistently appCfg := config.Config{} appCfg.Url.VirtualWorkspacePrefix = "virtual-workspace" From ba51492d31c3402641bf4754b98c85af6ae637d7 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 11:43:36 +0200 Subject: [PATCH 18/22] fixed getOperationFromContext bug On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index 44bc13b2..bd0b96db 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -5,7 +5,6 @@ import ( "strings" "github.com/graphql-go/graphql" - "go.opentelemetry.io/otel/trace" "golang.org/x/text/cases" "golang.org/x/text/language" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -159,14 +158,7 @@ func (r *Service) getOperationFromContext(ctx context.Context) string { return op } - // Fallback: try to extract from trace span name - span := trace.SpanFromContext(ctx) - if span == nil { - return "unknown" - } - - // This is a workaround - we'll need to get the span name somehow - // For now, assume unknown and rely on context values + // No reliable way to extract operation type from context alone return "unknown" } From 8897cdf2751f824fea9ff92f9c13cae2220fb24c Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 14:12:22 +0200 Subject: [PATCH 19/22] removed unworked code On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/relations.go | 53 +++++++++---------- gateway/resolver/resolver.go | 34 ++++++------ gateway/resolver/subscription.go | 9 ---- listener/pkg/apischema/builder.go | 42 ++++----------- .../reconciler/kcp/virtual_workspace_test.go | 4 +- 5 files changed, 52 insertions(+), 90 deletions(-) diff --git a/gateway/resolver/relations.go b/gateway/resolver/relations.go index bd0b96db..4c548542 100644 --- a/gateway/resolver/relations.go +++ b/gateway/resolver/relations.go @@ -25,11 +25,8 @@ type referenceInfo struct { // Relationships are only enabled for GetItem queries to prevent N+1 problems in ListItems and Subscriptions func (r *Service) RelationResolver(fieldName string, gvk schema.GroupVersionKind) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { - // Try context first, fallback to GraphQL info analysis - operation := r.getOperationFromContext(p.Context) - if operation == "unknown" { - operation = r.detectOperationFromGraphQLInfo(p) - } + // Determine operation type from GraphQL path analysis + operation := r.detectOperationFromGraphQLInfo(p) r.log.Debug(). Str("fieldName", fieldName). @@ -135,9 +132,9 @@ func (r *Service) resolveReference(ctx context.Context, ref referenceInfo, targe func (r *Service) isRelationResolutionAllowedForOperation(operation string) bool { // Only allow relationships for GetItem and GetItemAsYAML operations switch operation { - case "GetItem", "GetItemAsYAML": + case GET_ITEM, GET_ITEM_AS_YAML: return true - case "ListItems", "SubscribeItem", "SubscribeItems": + case LIST_ITEMS, SUBSCRIBE_ITEM, SUBSCRIBE_ITEMS: return false default: // For unknown operations, be conservative and disable relationships @@ -146,22 +143,6 @@ func (r *Service) isRelationResolutionAllowedForOperation(operation string) bool } } -// Context key for tracking operation type -type operationContextKey string - -const OperationTypeKey operationContextKey = "operation_type" - -// getOperationFromContext extracts the operation name from the context -func (r *Service) getOperationFromContext(ctx context.Context) string { - // Try to get operation from context value first - if op, ok := ctx.Value(OperationTypeKey).(string); ok { - return op - } - - // No reliable way to extract operation type from context alone - return "unknown" -} - // detectOperationFromGraphQLInfo analyzes GraphQL field path to determine operation type // This looks at the parent field context to determine if we're in a list, single item, or subscription func (r *Service) detectOperationFromGraphQLInfo(p graphql.ResolveParams) string { @@ -183,7 +164,23 @@ func (r *Service) detectOperationFromGraphQLInfo(p graphql.ResolveParams) string r.log.Debug(). Str("parentField", fieldName). Msg("Detected subscription context from parent field") - return "SubscribeItems" + return SUBSCRIBE_ITEMS + } + + // Check for mutation patterns + if strings.HasPrefix(fieldLower, "create") { + return CREATE_ITEM + } + if strings.HasPrefix(fieldLower, "update") { + return UPDATE_ITEM + } + if strings.HasPrefix(fieldLower, "delete") { + return DELETE_ITEM + } + + // Check for YAML patterns + if strings.HasSuffix(fieldLower, "yaml") { + return GET_ITEM_AS_YAML } // Check for list patterns (plural without args, or explicitly plural fields) @@ -192,15 +189,15 @@ func (r *Service) detectOperationFromGraphQLInfo(p graphql.ResolveParams) string r.log.Debug(). Str("parentField", fieldName). Msg("Detected list context from parent field") - return "ListItems" + return LIST_ITEMS } } } // If we can't determine from parent context, assume it's a single item operation - // This is the safe default that allows relationships + // This is the safe default that allows relationships for queries r.log.Debug(). Str("currentField", p.Info.FieldName). - Msg("Could not determine operation from path, defaulting to GetItem") - return "GetItem" + Msg("Could not determine operation type from GraphQL path, defaulting to GetItem (enables relations)") + return GET_ITEM } diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index 9dc8484b..045a1615 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -2,7 +2,6 @@ package resolver import ( "bytes" - "context" "encoding/json" "errors" "fmt" @@ -25,6 +24,17 @@ import ( "github.com/openmfp/golang-commons/logger" ) +const ( + LIST_ITEMS = "ListItems" + GET_ITEM = "GetItem" + GET_ITEM_AS_YAML = "GetItemAsYAML" + CREATE_ITEM = "CreateItem" + UPDATE_ITEM = "UpdateItem" + DELETE_ITEM = "DeleteItem" + SUBSCRIBE_ITEM = "SubscribeItem" + SUBSCRIBE_ITEMS = "SubscribeItems" +) + type Provider interface { CrudProvider CustomQueriesProvider @@ -66,14 +76,9 @@ func New(log *logger.Logger, runtimeClient client.WithWatch) *Service { // ListItems returns a GraphQL CommonResolver function that lists Kubernetes resources of the given GroupVersionKind. func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) graphql.FieldResolveFn { return func(p graphql.ResolveParams) (interface{}, error) { - ctx, span := otel.Tracer("").Start(p.Context, "ListItems", trace.WithAttributes(attribute.String("kind", gvk.Kind))) + ctx, span := otel.Tracer("").Start(p.Context, LIST_ITEMS, trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - // Add operation type to context to disable relationship resolution - ctx = context.WithValue(ctx, operationContextKey("operation_type"), "ListItems") - // Update p.Context so field resolvers inherit the operation type - p.Context = ctx - gvk.Group = r.getOriginalGroupName(gvk.Group) log, err := r.log.ChildLoggerWithAttributes( @@ -94,10 +99,11 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) var opts []client.ListOption - if val, ok := p.Args[LabelSelectorArg].(string); ok && val != "" { - selector, err := labels.Parse(val) + // Handle label selector argument + if labelSelector, ok := p.Args[LabelSelectorArg].(string); ok && labelSelector != "" { + selector, err := labels.Parse(labelSelector) if err != nil { - log.Error().Err(err).Str(LabelSelectorArg, val).Msg("Unable to parse given label selector") + log.Error().Err(err).Str(LabelSelectorArg, labelSelector).Msg("Unable to parse given label selector") return nil, err } opts = append(opts, client.MatchingLabelsSelector{Selector: selector}) @@ -148,11 +154,6 @@ func (r *Service) GetItem(gvk schema.GroupVersionKind, scope v1.ResourceScope) g ctx, span := otel.Tracer("").Start(p.Context, "GetItem", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - // Add operation type to context to enable relationship resolution - ctx = context.WithValue(ctx, operationContextKey("operation_type"), "GetItem") - // Update p.Context so field resolvers inherit the operation type - p.Context = ctx - gvk.Group = r.getOriginalGroupName(gvk.Group) log, err := r.log.ChildLoggerWithAttributes( @@ -206,9 +207,6 @@ func (r *Service) GetItemAsYAML(gvk schema.GroupVersionKind, scope v1.ResourceSc p.Context, span = otel.Tracer("").Start(p.Context, "GetItemAsYAML", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - // Add operation type to context to enable relationship resolution - p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "GetItemAsYAML") - out, err := r.GetItem(gvk, scope)(p) if err != nil { return "", err diff --git a/gateway/resolver/subscription.go b/gateway/resolver/subscription.go index 8601938c..faa8d041 100644 --- a/gateway/resolver/subscription.go +++ b/gateway/resolver/subscription.go @@ -1,7 +1,6 @@ package resolver import ( - "context" "fmt" "reflect" "sort" @@ -34,9 +33,6 @@ func (r *Service) SubscribeItem(gvk schema.GroupVersionKind, scope v1.ResourceSc _, span := otel.Tracer("").Start(p.Context, "SubscribeItem", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - // Add operation type to context to disable relationship resolution - p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "SubscribeItem") - resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, true, scope) @@ -49,9 +45,6 @@ func (r *Service) SubscribeItems(gvk schema.GroupVersionKind, scope v1.ResourceS _, span := otel.Tracer("").Start(p.Context, "SubscribeItems", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - // Add operation type to context to disable relationship resolution - p.Context = context.WithValue(p.Context, operationContextKey("operation_type"), "SubscribeItems") - resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, false, scope) @@ -362,8 +355,6 @@ func CreateSubscriptionResolver(isSingle bool) graphql.FieldResolveFn { return nil, err } - // Note: The context already contains operation type from SubscribeItem/SubscribeItems - // This will propagate to relationship resolvers, disabling them for subscriptions return source, nil } } diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 1c1b2bab..b70228c7 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -238,13 +238,9 @@ func (b *SchemaBuilder) WithRelationships() *SchemaBuilder { // Build kind registry first b.buildKindRegistry() - // For depth=1: use simple relation target tracking (working approach) - // For depth>1: use iterative expansion (scalable approach) - if b.maxRelationDepth == 1 { - b.expandWithSimpleDepthControl() - } else { - b.expandWithConfigurableDepthControl() - } + // Currently only supports depth=1 relation expansion + // TODO: Implement configurable depth control for depth > 1 when needed + b.expandWithSimpleDepthControl() return b } @@ -286,18 +282,6 @@ func (b *SchemaBuilder) expandWithSimpleDepthControl() { } } -// expandWithConfigurableDepthControl implements scalable depth control for depth > 1 -func (b *SchemaBuilder) expandWithConfigurableDepthControl() { - b.log.Info(). - Int("kindRegistrySize", len(b.kindRegistry)). - Int("maxRelationDepth", b.maxRelationDepth). - Msg("Starting configurable relationship expansion") - - // TODO: Implement proper multi-level depth control - // For now, fall back to simple approach - b.expandWithSimpleDepthControl() -} - // buildKindRegistry builds a map of kind names to available resource types func (b *SchemaBuilder) buildKindRegistry() { for schemaKey, schema := range b.schemas { @@ -313,12 +297,14 @@ func (b *SchemaBuilder) buildKindRegistry() { jsonBytes, err := json.Marshal(gvksVal) if err != nil { + b.err = multierror.Append(b.err, errors.Join(ErrBuildKindRegistry, err)) b.log.Debug().Err(err).Str("schemaKey", schemaKey).Msg("failed to marshal GVK") continue } var gvks []*GroupVersionKind if err := json.Unmarshal(jsonBytes, &gvks); err != nil { + b.err = multierror.Append(b.err, errors.Join(ErrBuildKindRegistry, err)) b.log.Debug().Err(err).Str("schemaKey", schemaKey).Msg("failed to unmarshal GVK") continue } @@ -450,7 +436,10 @@ func (b *SchemaBuilder) findBestResourceForKind(kindName string) *ResourceInfo { best := b.selectByKubectlPriority(candidates) // Log warning about the conflict for observability - groups := b.formatCandidateGroups(candidates) + groups := make([]string, len(candidates)) + for i, candidate := range candidates { + groups[i] = b.formatGroupVersion(candidate) + } b.log.Warn(). Str("kind", kindName). Str("selectedGroup", b.formatGroupVersion(best)). @@ -554,19 +543,6 @@ func (b *SchemaBuilder) addRelationshipField(schema *spec.Schema, schemaKey, pro Msg("Added relationship field") } -// formatCandidateGroups formats candidate groups for error messages -func (b *SchemaBuilder) formatCandidateGroups(candidates []ResourceInfo) []string { - groups := make([]string, len(candidates)) - for i, candidate := range candidates { - if candidate.Group == "" { - groups[i] = fmt.Sprintf("core/%s", candidate.Version) - } else { - groups[i] = fmt.Sprintf("%s/%s", candidate.Group, candidate.Version) - } - } - return groups -} - func isRefProperty(name string) bool { if !strings.HasSuffix(name, "Ref") { return false diff --git a/listener/reconciler/kcp/virtual_workspace_test.go b/listener/reconciler/kcp/virtual_workspace_test.go index 8af7a80f..babf7387 100644 --- a/listener/reconciler/kcp/virtual_workspace_test.go +++ b/listener/reconciler/kcp/virtual_workspace_test.go @@ -664,7 +664,7 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { URL: "https://example.com", }, expectError: true, // Expected due to kubeconfig dependency in metadata injection - expectedWriteCalls: 0, // Won't reach write due to metadata injection failure in CI + expectedWriteCalls: 0, // Won't reach write due to metadata injection failure errorShouldContain: "failed to inject KCP cluster metadata", }, { @@ -675,7 +675,7 @@ func TestVirtualWorkspaceReconciler_ProcessVirtualWorkspace(t *testing.T) { }, ioWriteError: errors.New("write failed"), expectError: true, // Expected due to kubeconfig dependency in metadata injection - expectedWriteCalls: 0, // Won't reach write due to metadata injection failure in CI + expectedWriteCalls: 0, // Won't reach write due to metadata injection failure errorShouldContain: "failed to inject KCP cluster metadata", }, { From ce396e42315d49adc915b57cb42ca85fa5824176 Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 14:47:44 +0200 Subject: [PATCH 20/22] reduced diff On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- gateway/resolver/resolver.go | 9 +++++---- gateway/resolver/subscription.go | 2 -- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/gateway/resolver/resolver.go b/gateway/resolver/resolver.go index 045a1615..c8f813f1 100644 --- a/gateway/resolver/resolver.go +++ b/gateway/resolver/resolver.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/graphql-go/graphql" + pkgErrors "github.com/pkg/errors" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -93,9 +94,9 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) log = r.log } - // Create a list of unstructured objects to hold the results + // Create an unstructured list to hold the results list := &unstructured.UnstructuredList{} - list.SetGroupVersionKind(schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind + "List"}) + list.SetGroupVersionKind(gvk) var opts []client.ListOption @@ -120,8 +121,8 @@ func (r *Service) ListItems(gvk schema.GroupVersionKind, scope v1.ResourceScope) } if err = r.runtimeClient.List(ctx, list, opts...); err != nil { - log.Error().Err(err).Str("scope", string(scope)).Msg("Unable to list objects") - return nil, err + log.Error().Err(err).Msg("Unable to list objects") + return nil, pkgErrors.Wrap(err, "unable to list objects") } sortBy, err := getStringArg(p.Args, SortByArg, false) diff --git a/gateway/resolver/subscription.go b/gateway/resolver/subscription.go index faa8d041..3b0216d4 100644 --- a/gateway/resolver/subscription.go +++ b/gateway/resolver/subscription.go @@ -32,7 +32,6 @@ func (r *Service) SubscribeItem(gvk schema.GroupVersionKind, scope v1.ResourceSc return func(p graphql.ResolveParams) (interface{}, error) { _, span := otel.Tracer("").Start(p.Context, "SubscribeItem", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, true, scope) @@ -44,7 +43,6 @@ func (r *Service) SubscribeItems(gvk schema.GroupVersionKind, scope v1.ResourceS return func(p graphql.ResolveParams) (interface{}, error) { _, span := otel.Tracer("").Start(p.Context, "SubscribeItems", trace.WithAttributes(attribute.String("kind", gvk.Kind))) defer span.End() - resultChannel := make(chan interface{}) go r.runWatch(p, gvk, resultChannel, false, scope) From 09b357b5f0c1adc641c489b0de5cc3230fe33dff Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 14:58:50 +0200 Subject: [PATCH 21/22] improved WithCRDCategories On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index b70228c7..65c52e18 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -155,24 +155,25 @@ func (b *SchemaBuilder) WithCRDCategories(crd *apiextensionsv1.CustomResourceDef return b } - gkv, err := getCRDGroupVersionKind(crd.Spec) + categories := crd.Spec.Names.Categories + if len(categories) == 0 { + return b + } + + gvk, err := getCRDGroupVersionKind(crd.Spec) if err != nil { - b.err = multierror.Append(b.err, ErrGetCRDGVK) + b.err = multierror.Append(b.err, errors.Join(ErrGetCRDGVK, err)) return b } for _, v := range crd.Spec.Versions { - resourceKey := getOpenAPISchemaKey(metav1.GroupVersionKind{Group: gkv.Group, Version: v.Name, Kind: gkv.Kind}) + resourceKey := getOpenAPISchemaKey(metav1.GroupVersionKind{Group: gvk.Group, Version: v.Name, Kind: gvk.Kind}) resourceSchema, ok := b.schemas[resourceKey] if !ok { continue } - if len(crd.Spec.Names.Categories) == 0 { - b.log.Debug().Str("resource", resourceKey).Msg("no categories provided for CRD kind") - continue - } - resourceSchema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, crd.Spec.Names.Categories) + resourceSchema.VendorExtensible.AddExtension(common.CategoriesExtensionKey, categories) b.schemas[resourceKey] = resourceSchema } return b From b6d83af1cc950b80bcea6dc07ddde4ee749b26fc Mon Sep 17 00:00:00 2001 From: Artem Shcherbatiuk Date: Thu, 21 Aug 2025 15:18:15 +0200 Subject: [PATCH 22/22] simplified depth control On-behalf-of: @SAP a.shcherbatiuk@sap.com Signed-off-by: Artem Shcherbatiuk --- listener/pkg/apischema/builder.go | 24 ++------------------ listener/pkg/apischema/relationships_test.go | 21 ----------------- 2 files changed, 2 insertions(+), 43 deletions(-) diff --git a/listener/pkg/apischema/builder.go b/listener/pkg/apischema/builder.go index 65c52e18..dd8d1550 100644 --- a/listener/pkg/apischema/builder.go +++ b/listener/pkg/apischema/builder.go @@ -39,8 +39,6 @@ type SchemaBuilder struct { log *logger.Logger kindRegistry map[GroupVersionKind]ResourceInfo // Changed: Use GVK as key for precise lookup preferredVersions map[string]string // map[group/kind]preferredVersion - maxRelationDepth int // maximum allowed relationship nesting depth (1 = single level) - relationDepths map[string]int // tracks the minimum depth at which each schema is referenced } // ResourceInfo holds information about a resource for relationship resolution @@ -56,8 +54,6 @@ func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logge schemas: make(map[string]*spec.Schema), kindRegistry: make(map[GroupVersionKind]ResourceInfo), preferredVersions: make(map[string]string), - maxRelationDepth: 1, // Default to 1-level depth for now - relationDepths: make(map[string]int), log: log, } @@ -79,19 +75,6 @@ func NewSchemaBuilder(oc openapi.Client, preferredApiGroups []string, log *logge return b } -// WithMaxRelationDepth sets the maximum allowed relationship nesting depth -// depth=1: A->B (single level) -// depth=2: A->B->C (two levels) -// depth=3: A->B->C->D (three levels) -func (b *SchemaBuilder) WithMaxRelationDepth(depth int) *SchemaBuilder { - if depth < 1 { - depth = 1 // Minimum depth is 1 - } - b.maxRelationDepth = depth - b.log.Info().Int("maxRelationDepth", depth).Msg("Set maximum relationship nesting depth") - return b -} - type GroupVersionKind struct { Group string `json:"group"` Version string `json:"version"` @@ -235,12 +218,12 @@ func (b *SchemaBuilder) WithPreferredVersions(apiResLists []*metav1.APIResourceL } // WithRelationships adds relationship fields to schemas that have *Ref fields +// Uses 1-level depth control to prevent circular references and N+1 problems func (b *SchemaBuilder) WithRelationships() *SchemaBuilder { // Build kind registry first b.buildKindRegistry() - // Currently only supports depth=1 relation expansion - // TODO: Implement configurable depth control for depth > 1 when needed + // Expand relationships with 1-level depth control b.expandWithSimpleDepthControl() return b @@ -341,9 +324,6 @@ func (b *SchemaBuilder) buildKindRegistry() { b.log.Debug().Int("gvkCount", len(b.kindRegistry)).Msg("built kind registry for relationships") } -// TODO: Implement proper multi-level depth calculation when needed -// For now, focusing on the working 1-level depth control - // warnAboutMissingPreferredVersions checks for kinds with multiple resources but no preferred versions func (b *SchemaBuilder) warnAboutMissingPreferredVersions() { // Group resources by kind name to find potential conflicts diff --git a/listener/pkg/apischema/relationships_test.go b/listener/pkg/apischema/relationships_test.go index 0521d8b0..f6bc5dae 100644 --- a/listener/pkg/apischema/relationships_test.go +++ b/listener/pkg/apischema/relationships_test.go @@ -178,27 +178,6 @@ func Test_depth_control_prevents_deep_nesting(t *testing.T) { assert.Equal(t, originalServiceProps, currentServiceProps, "Service should not have relationship fields added") } -func Test_configurable_depth_control_api(t *testing.T) { - mock := apimocks.NewMockClient(t) - mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil) - b := apischema.NewSchemaBuilder(mock, nil, testlogger.New().Logger) - - // Test that WithMaxRelationDepth API works - b.WithMaxRelationDepth(2) - - // Create simple schemas to verify the builder accepts the API - rootSchema := schemaWithGVK("example.com", "v1", "Root") - b.SetSchemas(map[string]*spec.Schema{ - "example.com.v1.Root": rootSchema, - }) - - // Should not panic or error - b.WithRelationships() - - // For now, even with depth=2, it falls back to depth=1 behavior (as per TODO) - // This test verifies the API works and doesn't break anything -} - func Test_single_level_prevents_circular_relationships(t *testing.T) { mock := apimocks.NewMockClient(t) mock.EXPECT().Paths().Return(map[string]openapi.GroupVersion{}, nil)