(bug) Fix post-render patch filtering for remove operations#625
Merged
Conversation
When using post-render patches, Sveltos filters out remove operations that target a path which doesn't exist on the object. This avoids errors when a patch is written defensively (e.g. "remove this label if it's there"). This PR fixes two issues in that logic: 1. JSON Pointer escape sequences were not decoded. 2. JSON Pointer paths use ~1 to represent a / character and ~0 to represent ~. A label key like velero.io/exclude-from-backup is written in a patch path as velero.io~1exclude-from-backup. The old code looked up that literal string as the map key, which never matched the actual key velero.io/exclude-from-backup. As a result, a remove operation targeting an existing label with a slash in its name was incorrectly treated as targeting a missing path and silently dropped. Array segments in paths were not handled. If a path traverses a list, for example /spec/containers/0/image, the old code tried to look up "0" as a map key, which always fails since list elements are not keyed by index. Any remove operation whose path passed through an array index was therefore always treated as missing and dropped. Multi-operation patches were only partially inspected. A single patch entry can contain multiple operations. Previously only the first operation was checked, so a remove on a missing path later in the same patch was never filtered and would cause an error at apply time. The fix inspects every operation individually: remove operations targeting missing paths are stripped, the rest are kept. If all operations survive, the patch is returned unchanged; if all are stripped, the patch is dropped entirely. Strategic merge patches without a resource name now produce a clear error. A strategic merge patch identifies its target resource by nam. It is essentially a partial resource object that gets merged into the real one. When such a patch has no metadata.name, kustomize rejects it with an opaque internal error that is hard to understand. This typically happens when a user writes a strategic merge patch expecting a regex target.name selector to handle the matching (which only works for JSON patches). Sveltos now detects this case early and returns a message that explains the problem and points to the JSON patch format as the solution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When using post-render patches, Sveltos filters out remove operations that target a path which doesn't exist on the object. This avoids errors when a patch is written defensively (e.g. "remove this label if it's there"). This PR fixes two issues in that logic:
~1to represent a / character and ~0 to represent~. A label key likevelero.io/exclude-from-backupis written in a patch path asvelero.io~1exclude-from-backup. The old code looked up that literal string as the map key, which never matched the actual key velero.io/exclude-from-backup. As a result, a remove operation targeting an existing label with a slash in its name was incorrectly treated as targeting a missing path and silently dropped.Array segments in paths were not handled.
If a path traverses a list, for example /spec/containers/0/image, the old code tried to look up "0" as a map key, which always fails since list elements are not keyed by index. Any remove operation whose path passed through an array index was therefore always treated as missing and dropped.
Multi-operation patches were only partially inspected. A single patch entry can contain multiple operations. Previously only the first operation was checked, so a remove on a missing path later in the same patch was never filtered and would cause an error at apply time. The fix inspects every operation individually: remove operations targeting missing paths are stripped, the rest are kept. If all operations survive, the patch is returned unchanged; if all are stripped, the patch is dropped entirely.
Strategic merge patches without a resource name now produce a clear error. A strategic merge patch identifies its target resource by nam. It is essentially a partial resource object that gets merged into the real one. When such a patch has no metadata.name, kustomize rejects it with an opaque internal error that is hard to understand. This typically happens when a user writes a strategic merge patch expecting a regex target.name selector to handle the matching (which only works for JSON patches). Sveltos now detects this case early and returns a message that explains the problem and points to the JSON patch format as the solution.