[Fix] Add SanitizePattern to trim whitespace and fix YAML manifest exports - #1037
[Fix] Add SanitizePattern to trim whitespace and fix YAML manifest exports#1037YASHMAHAKAL wants to merge 2 commits into
Conversation
…iguration When a Meshery design is exported as a Kubernetes manifest (or Helm chart), gopkg.in/yaml.v3 quotes map keys and string values that contain leading or trailing whitespace. This produces invalid-looking output such as: 'storage ': 2Gi (should be: storage: 2Gi) name: 'test-volume ' (should be: name: test-volume) The root cause is that whitespace entered via the RJSF form (particularly the additionalProperties key editor) is stored verbatim in comp.Configuration and then passed through the K8s converter unchanged. SanitizePattern recursively trims every string key and string value inside each component's Configuration map, and also trims DisplayName. Non-string leaves (bool, int, float64, nil) are passed through unchanged to preserve schema-typed fields. Callers: invoke SanitizePattern at the design-save boundary in meshery/server alongside DehydratePattern (handlePatternPOST). Signed-off-by: Yash Mahajan <mahajanyash.02@gmail.com> Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the SanitizePattern function and associated helper functions to recursively trim leading and trailing whitespace from component display names and configuration map keys and values, preventing unwanted YAML quoting issues. It also includes comprehensive unit tests for these changes. The review feedback highlights a potential nil pointer dereference in SanitizePattern if the input pattern or any of its components are nil, and suggests adding defensive checks to ensure robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…afety Add defensive nil checks to SanitizePattern: - Guard against nil *PatternFile pointer (p == nil -> return) - Guard against nil component pointers in p.Components slice (comp == nil -> continue) The current Meshery call site at handlePatternPOST always passes a non-nil pointer (&requestPayload.DesignFile), so these guards do not change behaviour for the existing consumer. They are added as a public library API contract so that any future caller is safe regardless of input. Added two new unit tests: - TestSanitizePattern_NilPatternFileIsNoop - TestSanitizePattern_NilComponentInSliceIsSkipped Signed-off-by: Yash Mahajan <mahajanyash.02@gmail.com> Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
|
Work outside of relationship definitions is on-hold for @YASHMAHAKAL. |
sure, i'll focus on relationship definitions for now |
yi-nuo426
left a comment
yi-nuo426
left a comment
There was a problem hiding this comment.
It seems like this could be done more efficiently and with less custom-written code.
There was a problem hiding this comment.
Pull request overview
This PR adds a patterns-level sanitization utility in MeshKit to prevent YAML export/serialization issues caused by incidental leading/trailing whitespace in design configuration keys/values (and component display names), along with unit tests validating the sanitizer’s behavior.
Changes:
- Added
SanitizePatternplus recursive helpers to trim whitespace in componentConfigurationmaps andDisplayName. - Implemented recursive traversal for nested
map[string]interface{}and[]interface{}while passing through non-string leaf types unchanged. - Added unit tests covering display name trimming, nested key/value trimming, nil/no-op behavior, and non-string leaf preservation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| models/patterns/pattern.go | Introduces SanitizePattern and recursive sanitization helpers for configuration maps/values. |
| models/patterns/pattern_sanitize_test.go | Adds unit tests to validate trimming behavior and type preservation. |
| func sanitizeConfigMap(m map[string]interface{}) map[string]interface{} { | ||
| if m == nil { | ||
| return nil | ||
| } | ||
| result := make(map[string]interface{}, len(m)) | ||
| for k, v := range m { | ||
| result[strings.TrimSpace(k)] = sanitizeConfigValue(v) | ||
| } | ||
| return result | ||
| } |
| func TestSanitizePattern_PreservesNonStringLeaves(t *testing.T) { | ||
| // bool, int, float, nil must pass through unchanged. | ||
| p := makePatternFile("comp", map[string]interface{}{ | ||
| "replicas": 3, | ||
| "enabled": true, | ||
| "ratio": 1.5, | ||
| "optionNil": nil, | ||
| }) | ||
| patterns.SanitizePattern(p) | ||
|
|
||
| cfg := p.Components[0].Configuration | ||
| if cfg["replicas"] != 3 { | ||
| t.Errorf("replicas changed: %v", cfg["replicas"]) | ||
| } | ||
| if cfg["enabled"] != true { | ||
| t.Errorf("enabled changed: %v", cfg["enabled"]) | ||
| } | ||
| if cfg["ratio"] != 1.5 { | ||
| t.Errorf("ratio changed: %v", cfg["ratio"]) | ||
| } | ||
| if cfg["optionNil"] != nil { | ||
| t.Errorf("optionNil changed: %v", cfg["optionNil"]) | ||
| } | ||
| } |
Description
This PR introduces a robust server-side sanitization utility designed to prevent YAML quoting artifacts in exported Kubernetes manifests.
When users accidentally type trailing whitespace in the UI,
yaml.v3strictly preserves it upon export by wrapping keys and values in single quotes (e.g.,'storage ': 1Gi), which causes the manifest to fail Kubernetes validation. This PR adds a data-scrubbing step that can be called at the persistence boundary to prevent this dirty data from ever reaching the database.Changes Made:
SanitizePatternalongside helper functions (sanitizeConfigMap,sanitizeConfigValue) to recursively traverse a pattern'sConfigurationmap.DisplayName.bool,int64,float64, andnilthrough unchanged to guarantee type safety and prevent schema data corruption.Related Issue:
Fixes #1036
Signed commits