Skip to content

Commit bd1b758

Browse files
jkoronaAtCiscoswiatekm
authored andcommitted
[chore][cmd/mdatagen] Fix config schema alias resolution (#15299)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Fixes alias resolution in internal/schemagen so config schemas resolve $defs aliases instead of emitting empty schema objects. This specifically fixes the scraper/scraperhelper case where: ```yaml controller_config: $ref: ./internal/controller.controller_config ``` could produce: ```json "controller_config": {} ``` Changes: - Keep root $defs lookup scoped to internal/local refs. - Avoid resolving a $defs entry to itself, so same-name local aliases fall through to the loader. - Prevent external refs from being shadowed by root $defs entries with the same definition name. - Add regression coverage for internal alias chains, same-name local aliases, and external ref shadowing. <!-- Issue number if applicable --> #### Link to tracking issue Fixes #15299
1 parent da747c8 commit bd1b758

2 files changed

Lines changed: 149 additions & 12 deletions

File tree

internal/schemagen/resolver.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -203,21 +203,11 @@ func (r *Resolver) resolveRef(root, current *ConfigMetadata, origin *Ref) (*Conf
203203
return nil, fmt.Errorf("invalid reference format %q: %w", current.Ref, err)
204204
}
205205

206-
if ref.IsInternal() {
207-
if root.Defs != nil {
208-
if val, ok := root.Defs[ref.DefName()]; ok {
209-
return val, nil
210-
}
211-
}
206+
if def, ok := lookupRootDef(root, current, ref); ok {
207+
return def, nil
212208
}
213209

214210
if ref.IsLocal() {
215-
// Local refs whose def is already in root.$defs (injected by the caller) can be resolved without loading a file.
216-
if root.Defs != nil {
217-
if val, ok := root.Defs[ref.DefName()]; ok {
218-
return val, nil
219-
}
220-
}
221211
return r.loadExternalRef(ref)
222212
}
223213

@@ -232,6 +222,17 @@ func (r *Resolver) resolveRef(root, current *ConfigMetadata, origin *Ref) (*Conf
232222
return current, nil
233223
}
234224

225+
func lookupRootDef(root, current *ConfigMetadata, ref *Ref) (*ConfigMetadata, bool) {
226+
if root.Defs == nil || (!ref.IsInternal() && !ref.IsLocal()) {
227+
return nil, false
228+
}
229+
def, ok := root.Defs[ref.DefName()]
230+
if !ok || def == current {
231+
return nil, false
232+
}
233+
return def, ok
234+
}
235+
235236
// loadExternalRef uses SchemaLoader to load external references
236237
func (r *Resolver) loadExternalRef(ref *Ref) (*ConfigMetadata, error) {
237238
md, err := r.loader.Load(*ref)

internal/schemagen/resolver_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,142 @@ func TestResolver_ResolveSchema_AliasChain_PreservesCustomExtensions(t *testing.
223223
require.NotNil(t, cfg.Properties["level"])
224224
}
225225

226+
func TestResolver_ResolveSchema_InternalAliasChain(t *testing.T) {
227+
resolver := &Resolver{
228+
pkgID: "go.opentelemetry.io/collector/test/component",
229+
class: "receiver",
230+
name: "test",
231+
loader: &mockLoader{schemas: map[string]*ConfigMetadata{}},
232+
}
233+
234+
src := &ConfigMetadata{
235+
Type: "object",
236+
Defs: map[string]*ConfigMetadata{
237+
"alias": {Ref: "base"},
238+
"base": {
239+
Type: "object",
240+
Description: "Base configuration",
241+
Properties: map[string]*ConfigMetadata{
242+
"endpoint": {Type: "string"},
243+
},
244+
},
245+
},
246+
Properties: map[string]*ConfigMetadata{
247+
"cfg": {
248+
Ref: "alias",
249+
},
250+
},
251+
}
252+
253+
result, err := resolver.ResolveSchema(src)
254+
require.NoError(t, err)
255+
256+
cfg := result.Properties["cfg"]
257+
require.NotNil(t, cfg)
258+
require.Equal(t, "base", cfg.ResolvedFrom)
259+
require.Equal(t, "object", cfg.Type)
260+
require.Equal(t, "Base configuration", cfg.Description)
261+
require.Contains(t, cfg.Properties, "endpoint")
262+
}
263+
264+
func TestResolver_ResolveSchema_DefsOnlyLocalAliasWithSameDefName(t *testing.T) {
265+
controllerSchema := &ConfigMetadata{
266+
Type: "object",
267+
Defs: map[string]*ConfigMetadata{
268+
"controller_config": {
269+
Type: "object",
270+
Description: "Controller configuration",
271+
Properties: map[string]*ConfigMetadata{
272+
"collection_interval": {Type: "string"},
273+
},
274+
},
275+
},
276+
}
277+
278+
resolver := &Resolver{
279+
pkgID: "go.opentelemetry.io/collector/scraper/scraperhelper",
280+
class: "pkg",
281+
name: "scraperhelper",
282+
loader: &mockLoader{schemas: map[string]*ConfigMetadata{
283+
"./internal/controller.controller_config": controllerSchema,
284+
}},
285+
}
286+
287+
src := &ConfigMetadata{
288+
Defs: map[string]*ConfigMetadata{
289+
"controller_config": {
290+
Ref: "./internal/controller.controller_config",
291+
},
292+
},
293+
}
294+
295+
result, err := resolver.ResolveSchema(src)
296+
require.NoError(t, err)
297+
298+
controller := result.Defs["controller_config"]
299+
require.NotNil(t, controller)
300+
require.Equal(t, "./internal/controller.controller_config", controller.ResolvedFrom)
301+
require.Equal(t, "object", controller.Type)
302+
require.Equal(t, "Controller configuration", controller.Description)
303+
require.Contains(t, controller.Properties, "collection_interval")
304+
}
305+
306+
func TestResolver_ResolveSchema_ExternalRefDoesNotUseRootDefWithSameName(t *testing.T) {
307+
externalSchema := &ConfigMetadata{
308+
Type: "object",
309+
Defs: map[string]*ConfigMetadata{
310+
"shared_config": {
311+
Type: "object",
312+
Description: "External shared config",
313+
Properties: map[string]*ConfigMetadata{
314+
"external": {Type: "string"},
315+
},
316+
},
317+
},
318+
}
319+
320+
ml := &mockLoader{
321+
schemas: map[string]*ConfigMetadata{
322+
"go.opentelemetry.io/collector/config/shared.shared_config": externalSchema,
323+
},
324+
}
325+
326+
resolver := &Resolver{
327+
pkgID: "go.opentelemetry.io/collector/test/component",
328+
class: "receiver",
329+
name: "test",
330+
loader: ml,
331+
}
332+
333+
src := &ConfigMetadata{
334+
Type: "object",
335+
Defs: map[string]*ConfigMetadata{
336+
"shared_config": {
337+
Type: "object",
338+
Description: "Local shared config",
339+
Properties: map[string]*ConfigMetadata{
340+
"local": {Type: "string"},
341+
},
342+
},
343+
},
344+
Properties: map[string]*ConfigMetadata{
345+
"cfg": {
346+
Ref: "go.opentelemetry.io/collector/config/shared.shared_config",
347+
},
348+
},
349+
}
350+
351+
result, err := resolver.ResolveSchema(src)
352+
require.NoError(t, err)
353+
354+
cfg := result.Properties["cfg"]
355+
require.NotNil(t, cfg)
356+
require.Equal(t, "go.opentelemetry.io/collector/config/shared.shared_config", cfg.ResolvedFrom)
357+
require.Equal(t, "External shared config", cfg.Description)
358+
require.Contains(t, cfg.Properties, "external")
359+
require.NotContains(t, cfg.Properties, "local")
360+
}
361+
226362
func TestResolver_ResolveSchema_NestedStructures(t *testing.T) {
227363
resolver := &Resolver{
228364
pkgID: "go.opentelemetry.io/collector/test/component",

0 commit comments

Comments
 (0)