diff --git a/docs/followups/M8.md b/docs/followups/M8.md index cd5bc75d..0a2cd3bc 100644 --- a/docs/followups/M8.md +++ b/docs/followups/M8.md @@ -8,14 +8,13 @@ items still open here move to "Open — opportunistic" below or to the milestone-tracked "Carry-forward from M8" section of `MILESTONES.md`. -- [ ] **YAML `KnownFields(true)` for component configs** — top-level - YAML loader rejects unknown fields; receiver-internal configs - decoded via `node.Decode(cfgInst)` do not. An operator can - typo `collection_intrval: 15s` and `tracecore validate` - accepts it. Affects every receiver, not just dcgm. *Trigger:* - thread `KnownFields(true)` into the per-component decoder in - `internal/pipelinebuilder/resolveComponent`. ~30 lines + a - regression test per receiver. +- *Closed: `internal/pipelinebuilder/resolveComponent` now re-decodes + each component config via `strictDecodeNode` (yaml.Decoder with + `KnownFields(true)`). Typo'd keys like `collection_intrval` + surface as ` "" decode: ... field not found in + type`. Regression covered by + `TestBuildPipelines_RejectsUnknownComponentField` across + receiver / processor / exporter roles.* - [ ] **Per-metric (not per-group) config toggles** — operators may want utilization on but codec.utilization off. M8 ships per-group toggles only; per-metric is a refinement. *Trigger:* diff --git a/internal/pipelinebuilder/builder.go b/internal/pipelinebuilder/builder.go index 11356da8..d2ab61fd 100644 --- a/internal/pipelinebuilder/builder.go +++ b/internal/pipelinebuilder/builder.go @@ -8,6 +8,7 @@ package pipelinebuilder import ( + "bytes" "context" "errors" "fmt" @@ -235,7 +236,7 @@ func resolveComponent( // named error rather than a nil-deref inside node.Decode. return componentSet{}, fmt.Errorf("pipeline %s: %s %q: factory returned nil default config", pID, role, name) } - if err := node.Decode(cfgInst); err != nil { + if err := strictDecodeNode(&node, cfgInst); err != nil { return componentSet{}, fmt.Errorf("pipeline %s: %s %q decode: %w", pID, role, name, err) } if err := cfgInst.Validate(); err != nil { @@ -427,3 +428,20 @@ func buildReceivers[C any]( } return receivers, nil } + +// strictDecodeNode decodes a yaml.Node into out with KnownFields(true) +// so a typo'd key (e.g. `collection_intrval`) fails at config-load +// time. yaml.Node.Decode bypasses the top-level decoder's strict-mode, +// so we re-marshal and feed it through a yaml.Decoder. +func strictDecodeNode(node *yaml.Node, out any) error { + raw, err := yaml.Marshal(node) + if err != nil { + return fmt.Errorf("re-marshal yaml node: %w", err) + } + dec := yaml.NewDecoder(bytes.NewReader(raw)) + dec.KnownFields(true) + if err := dec.Decode(out); err != nil { + return fmt.Errorf("strict decode: %w", err) + } + return nil +} diff --git a/internal/pipelinebuilder/builder_test.go b/internal/pipelinebuilder/builder_test.go index 35691bc2..775cd8df 100644 --- a/internal/pipelinebuilder/builder_test.go +++ b/internal/pipelinebuilder/builder_test.go @@ -64,6 +64,74 @@ func TestBuildPipelines_WithProcessor(t *testing.T) { "buildProcessors must construct the noop processor instance") } +// TestBuildPipelines_RejectsUnknownComponentField confirms a typo'd +// key in any component config (receiver / processor / exporter) is +// rejected at build time. Catches `collection_intrval: 15s` etc. +// without each receiver needing its own strict-decode wiring. +func TestBuildPipelines_RejectsUnknownComponentField(t *testing.T) { + t.Parallel() + + mkCfg := func(receiver, processor, exporter string) *config.Config { + return &config.Config{ + Receivers: map[string]yaml.Node{"typed": mustYAML(receiver)}, + Processors: map[string]yaml.Node{"noop": mustYAML(processor)}, + Exporters: map[string]yaml.Node{"sink": mustYAML(exporter)}, + Service: config.Service{ + Pipelines: map[string]config.Pipeline{ + "metrics/primary": { + Receivers: []string{"typed"}, + Processors: []string{"noop"}, + Exporters: []string{"sink"}, + }, + }, + }, + } + } + factories := pipeline.Factories{ + Receivers: map[pipeline.Type]pipeline.ReceiverFactory{ + pipeline.MustNewType("typed"): &typedReceiverFactory{}, + }, + Processors: map[pipeline.Type]pipeline.ProcessorFactory{ + pipeline.MustNewType("noop"): &noopProcessorFactory{}, + }, + Exporters: map[pipeline.Type]pipeline.ExporterFactory{ + pipeline.MustNewType("sink"): &sinkExporterFactory{}, + }, + } + + t.Run("receiver typo", func(t *testing.T) { + t.Parallel() + cfg := mkCfg(`{collection_intrval: 15s}`, `{}`, `{}`) + _, err := pipelinebuilder.BuildPipelines(t.Context(), discardLogger(), cfg, factories) + require.Error(t, err) + require.Contains(t, err.Error(), `receiver "typed" decode`) + require.Contains(t, err.Error(), "collection_intrval") + }) + + t.Run("processor typo", func(t *testing.T) { + t.Parallel() + cfg := mkCfg(`{collection_interval: 15s}`, `{not_a_field: 1}`, `{}`) + _, err := pipelinebuilder.BuildPipelines(t.Context(), discardLogger(), cfg, factories) + require.Error(t, err) + require.Contains(t, err.Error(), `processor "noop" decode`) + }) + + t.Run("exporter typo", func(t *testing.T) { + t.Parallel() + cfg := mkCfg(`{collection_interval: 15s}`, `{}`, `{bogus: true}`) + _, err := pipelinebuilder.BuildPipelines(t.Context(), discardLogger(), cfg, factories) + require.Error(t, err) + require.Contains(t, err.Error(), `exporter "sink" decode`) + }) + + t.Run("valid config still builds", func(t *testing.T) { + t.Parallel() + cfg := mkCfg(`{collection_interval: 15s}`, `{}`, `{}`) + _, err := pipelinebuilder.BuildPipelines(t.Context(), discardLogger(), cfg, factories) + require.NoError(t, err) + }) +} + func mustYAML(s string) yaml.Node { if s == "" { panic("mustYAML: empty input — pass at least an empty mapping {}") @@ -145,6 +213,33 @@ type emptyConfig struct{} func (*emptyConfig) Validate() error { return nil } +// typedConfig has a real field so KnownFields(true) rejects typos +// like `collection_intrval` while accepting `collection_interval`. +type typedConfig struct { + CollectionInterval string `yaml:"collection_interval"` +} + +func (*typedConfig) Validate() error { return nil } + +type typedReceiverFactory struct{} + +func (*typedReceiverFactory) Type() pipeline.Type { return pipeline.MustNewType("typed") } +func (*typedReceiverFactory) CreateDefaultConfig() pipeline.Config { + return &typedConfig{} +} + +func (*typedReceiverFactory) CreateMetrics(_ context.Context, _ pipeline.CreateSettings, _ pipeline.Config, _ consumer.Metrics) (pipeline.Receiver, error) { + return noopComponent{}, nil +} + +func (*typedReceiverFactory) CreateTraces(_ context.Context, _ pipeline.CreateSettings, _ pipeline.Config, _ consumer.Traces) (pipeline.Receiver, error) { + return nil, pipeline.ErrSignalNotSupported +} + +func (*typedReceiverFactory) CreateLogs(_ context.Context, _ pipeline.CreateSettings, _ pipeline.Config, _ consumer.Logs) (pipeline.Receiver, error) { + return nil, pipeline.ErrSignalNotSupported +} + type noopComponent struct{} func (noopComponent) Start(context.Context, pipeline.Host) error { return nil }