Conversation
There was a problem hiding this comment.
Pull request overview
This PR enables nested property access in Traefik templates using dot notation (e.g., {{ userData.port }}) while maintaining backward compatibility with flat key access (e.g., {{ port }}). The changes also remove the warnings system from the validator since template variable errors are now caught earlier during rendering.
Changes:
- Added support for nested property access in templates via dot notation
- Removed warning system from validator (template errors now throw during rendering)
- Changed error handling for missing templates from throwing to returning null
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/backends/traefik/templateParser.ts | Updated regex patterns to allow dots, added getContextValue function for nested property resolution, modified buildContext to populate both flat keys and userData namespace |
| src/backends/traefik/validators.ts | Removed warnings from ValidationResult type and eliminated unreplaced template variable checks |
| src/backends/traefik/traefikManager.ts | Removed warning handling code that logged validation warnings |
| src/backends/traefik/traefik.ts | Changed template-not-found handling from throwing error to returning null |
| test/unit/traefik/template-validation.test.ts | Updated tests to reflect removal of warnings and document new behavior |
| /** | ||
| * Get a value from context, supporting nested property access with dot notation. | ||
| * e.g., "userData.foo" returns context.userData.foo | ||
| */ | ||
| function getContextValue(path: string): string | undefined { | ||
| const parts = path.split('.'); | ||
| let value: any = context; | ||
|
|
||
| for (const part of parts) { | ||
| if (value == null || typeof value !== 'object') { | ||
| return undefined; | ||
| } | ||
| value = value[part]; | ||
| } | ||
|
|
||
| return value == null ? undefined : String(value); | ||
| } |
There was a problem hiding this comment.
The new nested property access feature (e.g., {{ userData.port }}) is not covered by tests. While the PR updates existing tests to remove warnings-related checks, there are no tests that verify the new functionality of accessing userData properties via dot notation.
| const available = Array.from(templates.keys()).join(', ') || '(none)'; | ||
| log.error({ message: 'Template not found', data: { appName, template: data.template, available } }); | ||
| throw new Error(`Template '${data.template}' not found for app '${appName}'. Available: ${available}`); | ||
| return null; |
There was a problem hiding this comment.
This changes the error handling behavior from throwing an error to returning null when a template is not found. While this makes the behavior consistent with render errors (which also return null), it's a potentially risky change. Previously, a missing template would cause an exception that could be caught higher up the stack. Now it silently fails and just logs an error. This could mask configuration issues during startup or runtime. Consider whether this behavior change is intentional and if it should be documented.
| /** Pattern for valid variable names (alphanumeric, underscores, and dots for nested access) */ | ||
| const VALID_KEY_PATTERN = /^[a-zA-Z0-9_.]+$/; |
There was a problem hiding this comment.
The VALID_KEY_PATTERN now allows dots in key names, which creates ambiguity. If a user provides userData with a key containing a dot (e.g., "foo.bar": "value"), the template variable {{ foo.bar }} will try to access it as a nested path (context.foo.bar) rather than a flat key (context["foo.bar"]). This means keys with dots in userData won't be accessible via the old flat syntax, only via {{ userData.foo.bar }} (which would also fail because it tries to access userData["foo"]["bar"]). Consider either disallowing dots in userData keys or documenting this limitation clearly.
| /** Pattern for valid variable names (alphanumeric, underscores, and dots for nested access) */ | |
| const VALID_KEY_PATTERN = /^[a-zA-Z0-9_.]+$/; | |
| /** Pattern for valid userData keys (alphanumeric and underscores only; no dots to avoid path ambiguity) */ | |
| const VALID_KEY_PATTERN = /^[a-zA-Z0-9_]+$/; |
1 participant
No description provided.