Skip to content

Commit 0244edb

Browse files
Copilotintel352
andauthored
template: warn on missing key access; add strict_templates opt-in (#372)
* Initial plan * Fix template missingkey=zero: add Option C (warning) and Option A (strict mode) Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/a9d0671f-a3b4-4962-9915-5e48a9e14c70 * template: fix step/trigger helpers in strict mode, remove template from warn log Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/cf41e59f-d4e3-409b-9717-d16b8e2d68b4 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Co-authored-by: Jonathan Langevin <codingsloth@pm.me>
1 parent e081b71 commit 0244edb

25 files changed

Lines changed: 416 additions & 129 deletions

DOCUMENTATION.md

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,36 @@ value: '{{ step "parse-request" "path_params" "id" }}'
429429
value: '{{ index .steps "parse-request" "path_params" "id" }}'
430430
```
431431

432-
`wfctl template validate --config workflow.yaml` lints template expressions and warns on undefined step references, forward references, and suggests the `step` function for hyphenated names.
432+
#### Missing Key Behaviour
433433

434+
By default, template expressions that reference a missing map key (e.g. a typo in a step field name) resolve to the zero value silently — but the engine now logs a **WARN** message to make the problem visible:
435+
436+
```
437+
WARN template resolved missing key to zero value pipeline=my-pipeline error="..."
438+
```
439+
440+
To turn the warning into a hard error, set `strict_templates: true` on the pipeline:
441+
442+
```yaml
443+
pipelines:
444+
my-pipeline:
445+
strict_templates: true # any missing key access fails the pipeline step
446+
steps:
447+
- name: process
448+
type: step.set
449+
config:
450+
values:
451+
tenant: "{{ .steps.auth.affilate_id }}" # typo: affilate_id instead of affiliate_id → step fails immediately
452+
```
453+
454+
| Mode | `strict_templates` | Missing key result |
455+
|------|-------------------|--------------------|
456+
| Default | `false` | Zero value (`<no value>`) + WARN log |
457+
| Strict | `true` | Step returns an error |
458+
459+
Strict mode applies to **both** direct dot-access (`{{ .steps.auth.field }}`) and the `step`/`trigger` helper functions (`{{ step "auth" "field" }}`). A missing key via either syntax will fail the step when `strict_templates: true` is set.
460+
461+
`wfctl template validate --config workflow.yaml` lints template expressions and warns on undefined step references and forward references. Use `strict_templates: true` in the pipeline config to catch field-level typos at runtime.
434462
### Infrastructure
435463
| Type | Description | Plugin |
436464
|------|-------------|--------|

config/pipeline.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ type PipelineConfig struct {
77
OnError string `json:"on_error,omitempty" yaml:"on_error,omitempty"`
88
Timeout string `json:"timeout,omitempty" yaml:"timeout,omitempty"`
99
Compensation []PipelineStepConfig `json:"compensation,omitempty" yaml:"compensation,omitempty"`
10+
// StrictTemplates causes the pipeline to return an error when any template
11+
// expression references a missing map key, instead of silently using the zero
12+
// value. Useful for catching typos in step field references at runtime.
13+
// Default is false (missing keys produce a warning log and resolve to zero).
14+
StrictTemplates bool `json:"strict_templates,omitempty" yaml:"strict_templates,omitempty"`
1015
}
1116

1217
// PipelineTriggerConfig defines what starts a pipeline.

engine.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ type StdEngine struct {
9393
// configHash is the SHA-256 hash of the last config built via BuildFromConfig.
9494
// Format: "sha256:<hex>". Empty until BuildFromConfig is called.
9595
configHash string
96-
9796
}
9897

9998
// App returns the underlying modular.Application.
@@ -142,7 +141,6 @@ func (e *StdEngine) SetPluginInstaller(installer *plugin.PluginInstaller) {
142141
e.pluginInstaller = installer
143142
}
144143

145-
146144
// NewStdEngine creates a new workflow engine
147145
func NewStdEngine(app modular.Application, logger modular.Logger) *StdEngine {
148146
e := &StdEngine{
@@ -854,11 +852,12 @@ func (e *StdEngine) configurePipelines(pipelineCfg map[string]any) error {
854852
}
855853

856854
pipeline := &module.Pipeline{
857-
Name: pipelineName,
858-
Steps: steps,
859-
OnError: onError,
860-
Timeout: timeout,
861-
Compensation: compSteps,
855+
Name: pipelineName,
856+
Steps: steps,
857+
OnError: onError,
858+
Timeout: timeout,
859+
Compensation: compSteps,
860+
StrictTemplates: pipeCfg.StrictTemplates,
862861
}
863862

864863
// Propagate the engine's logger to the pipeline so that execution logs

interfaces/iac_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (m *mockProvider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.
4444
return nil, nil
4545
}
4646
func (m *mockProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { return nil, nil }
47-
func (m *mockProvider) Close() error { return nil }
47+
func (m *mockProvider) Close() error { return nil }
4848

4949
// mockDriver implements ResourceDriver
5050
type mockDriver struct{}
@@ -84,7 +84,7 @@ func (s *mockState) GetResource(_ context.Context, _ string) (*interfaces.Resour
8484
func (s *mockState) ListResources(_ context.Context) ([]interfaces.ResourceState, error) {
8585
return nil, nil
8686
}
87-
func (s *mockState) DeleteResource(_ context.Context, _ string) error { return nil }
87+
func (s *mockState) DeleteResource(_ context.Context, _ string) error { return nil }
8888
func (s *mockState) SavePlan(_ context.Context, _ interfaces.IaCPlan) error { return nil }
8989
func (s *mockState) GetPlan(_ context.Context, _ string) (*interfaces.IaCPlan, error) {
9090
return nil, nil

interfaces/pipeline.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ type PipelineContext struct {
5757

5858
// Metadata holds execution metadata (pipeline name, trace ID, etc.)
5959
Metadata map[string]any
60+
61+
// StrictTemplates causes template execution to return an error instead of
62+
// the zero value when a template expression references a missing map key.
63+
// When false (the default), missing keys resolve to the zero value with a
64+
// warning logged via Logger. Enable via pipeline config: strict_templates: true.
65+
StrictTemplates bool
66+
67+
// Logger is used to emit warnings when a template expression resolves a
68+
// missing key to the zero value (non-strict mode). When nil, slog.Default()
69+
// is used.
70+
Logger *slog.Logger
6071
}
6172

6273
// NewPipelineContext creates a PipelineContext initialized with trigger data.

module/iac_state_azure_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ import (
1313

1414
// mockAzureClient is an in-memory implementation of AzureBlobClient for testing.
1515
type mockAzureClient struct {
16-
mu sync.Mutex
17-
blobs map[string][]byte // name -> body
18-
leases map[string]string // name -> leaseID (empty = not leased)
16+
mu sync.Mutex
17+
blobs map[string][]byte // name -> body
18+
leases map[string]string // name -> leaseID (empty = not leased)
1919
}
2020

2121
func newMockAzureClient() *mockAzureClient {

module/iac_state_gcs_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import (
1414
// mockGCSClient is an in-memory implementation of GCSObjectClient for testing.
1515
type mockGCSClient struct {
1616
mu sync.Mutex
17-
objects map[string][]byte // key -> body
18-
generation map[string]int64 // key -> current generation
19-
errOnPut map[string]error // key -> error to return on conditional Put
17+
objects map[string][]byte // key -> body
18+
generation map[string]int64 // key -> current generation
19+
errOnPut map[string]error // key -> error to return on conditional Put
2020
}
2121

2222
func newMockGCSClient() *mockGCSClient {

module/infra_module_deploy_bridge_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (d *deployCapableDriver) Update(_ context.Context, image string) error {
4646
d.updateCalled = true
4747
return nil
4848
}
49-
func (d *deployCapableDriver) HealthCheck(_ context.Context, _ string) error { return d.healthErr }
49+
func (d *deployCapableDriver) HealthCheck(_ context.Context, _ string) error { return d.healthErr }
5050
func (d *deployCapableDriver) CurrentImage(_ context.Context) (string, error) { return d.image, nil }
5151
func (d *deployCapableDriver) ReplicaCount(_ context.Context) (int, error) { return d.replicas, nil }
5252

@@ -93,9 +93,9 @@ func (d *deployCapableDriver) DestroyCanary(_ context.Context) error {
9393

9494
type deployProviderMock struct {
9595
*infraMockProvider
96-
deployDriver *deployCapableDriver
97-
bgDriver *deployCapableDriver
98-
canaryDriver *deployCapableDriver
96+
deployDriver *deployCapableDriver
97+
bgDriver *deployCapableDriver
98+
canaryDriver *deployCapableDriver
9999
}
100100

101101
func (p *deployProviderMock) ProvideDeployDriver(_ string) DeployDriver {

module/infra_module_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@ func newInfraMockApp() *infraMockApp {
2020
return &infraMockApp{services: make(map[string]any)}
2121
}
2222

23-
func (a *infraMockApp) RegisterConfigSection(string, modular.ConfigProvider) {}
23+
func (a *infraMockApp) RegisterConfigSection(string, modular.ConfigProvider) {}
2424
func (a *infraMockApp) GetConfigSection(string) (modular.ConfigProvider, error) { return nil, nil }
2525
func (a *infraMockApp) ConfigSections() map[string]modular.ConfigProvider {
2626
return nil
2727
}
28-
func (a *infraMockApp) Logger() modular.Logger { return &noopLogger{} }
29-
func (a *infraMockApp) SetLogger(modular.Logger) {}
30-
func (a *infraMockApp) ConfigProvider() modular.ConfigProvider { return nil }
31-
func (a *infraMockApp) SvcRegistry() modular.ServiceRegistry { return a.services }
32-
func (a *infraMockApp) RegisterModule(modular.Module) {}
28+
func (a *infraMockApp) Logger() modular.Logger { return &noopLogger{} }
29+
func (a *infraMockApp) SetLogger(modular.Logger) {}
30+
func (a *infraMockApp) ConfigProvider() modular.ConfigProvider { return nil }
31+
func (a *infraMockApp) SvcRegistry() modular.ServiceRegistry { return a.services }
32+
func (a *infraMockApp) RegisterModule(modular.Module) {}
3333
func (a *infraMockApp) RegisterService(name string, svc any) error {
3434
a.services[name] = svc
3535
return nil
@@ -46,23 +46,23 @@ func (a *infraMockApp) GetService(name string, target any) error {
4646
}
4747
return nil
4848
}
49-
func (a *infraMockApp) Init() error { return nil }
50-
func (a *infraMockApp) Start() error { return nil }
51-
func (a *infraMockApp) Stop() error { return nil }
52-
func (a *infraMockApp) Run() error { return nil }
53-
func (a *infraMockApp) IsVerboseConfig() bool { return false }
54-
func (a *infraMockApp) SetVerboseConfig(bool) {}
55-
func (a *infraMockApp) Context() context.Context { return context.Background() }
56-
func (a *infraMockApp) GetServicesByModule(string) []string { return nil }
49+
func (a *infraMockApp) Init() error { return nil }
50+
func (a *infraMockApp) Start() error { return nil }
51+
func (a *infraMockApp) Stop() error { return nil }
52+
func (a *infraMockApp) Run() error { return nil }
53+
func (a *infraMockApp) IsVerboseConfig() bool { return false }
54+
func (a *infraMockApp) SetVerboseConfig(bool) {}
55+
func (a *infraMockApp) Context() context.Context { return context.Background() }
56+
func (a *infraMockApp) GetServicesByModule(string) []string { return nil }
5757
func (a *infraMockApp) GetServiceEntry(string) (*modular.ServiceRegistryEntry, bool) {
5858
return nil, false
5959
}
6060
func (a *infraMockApp) GetServicesByInterface(reflect.Type) []*modular.ServiceRegistryEntry {
6161
return nil
6262
}
63-
func (a *infraMockApp) GetModule(string) modular.Module { return nil }
64-
func (a *infraMockApp) GetAllModules() map[string]modular.Module { return nil }
65-
func (a *infraMockApp) StartTime() time.Time { return time.Time{} }
63+
func (a *infraMockApp) GetModule(string) modular.Module { return nil }
64+
func (a *infraMockApp) GetAllModules() map[string]modular.Module { return nil }
65+
func (a *infraMockApp) StartTime() time.Time { return time.Time{} }
6666
func (a *infraMockApp) OnConfigLoaded(func(modular.Application) error) {}
6767

6868
// infraMockProvider implements interfaces.IaCProvider for tests.
@@ -408,11 +408,11 @@ func TestInfraModule_ProvidesServices(t *testing.T) {
408408

409409
func TestInfraModule_ResourceConfig_StripsStandardKeys(t *testing.T) {
410410
m := NewInfraModule("db", "infra.database", map[string]any{
411-
"provider": "aws",
412-
"size": "m",
411+
"provider": "aws",
412+
"size": "m",
413413
"resources": map[string]any{"cpu": "2"},
414-
"engine": "postgres",
415-
"version": "16",
414+
"engine": "postgres",
415+
"version": "16",
416416
})
417417
cfg := m.ResourceConfig()
418418
if _, ok := cfg["provider"]; ok {

module/pipeline_executor.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ type Pipeline struct {
4040
// used by step.request_parse for path parameter extraction.
4141
RoutePattern string
4242

43+
// StrictTemplates enables strict template key resolution: when true, any
44+
// template expression that references a missing map key returns an error
45+
// instead of silently resolving to the zero value. Corresponds to the
46+
// pipeline config field strict_templates.
47+
StrictTemplates bool
48+
4349
// EventRecorder is an optional recorder for execution events.
4450
// When nil (the default), no events are recorded. Events are best-effort:
4551
// recording failures are logged but never fail the pipeline.
@@ -121,11 +127,13 @@ func (p *Pipeline) Execute(ctx context.Context, triggerData map[string]any) (*Pi
121127
}
122128
}
123129
pc := NewPipelineContext(triggerData, md)
130+
pc.StrictTemplates = p.StrictTemplates
124131

125132
logger := p.Logger
126133
if logger == nil {
127134
logger = slog.Default()
128135
}
136+
pc.Logger = logger
129137

130138
logger.Info("Pipeline started", "pipeline", p.Name, "steps", len(p.Steps))
131139

0 commit comments

Comments
 (0)