feat(differ): add feature gate for v4#498
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a full ChangesDifferV3 Implementation and Test Coverage
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DifferV3
participant diffResource
participant diffPlugins
participant mergeDefault
participant postprocessSubEvent
Caller->>DifferV3: diff(local, remote, defaultValue)
DifferV3->>DifferV3: new DifferV3({ transactionId: randomUUID(), defaultValue })
loop each ADCSDK.ResourceType
DifferV3->>diffResource: diffResource(resourceType, local[type], remote[type])
diffResource->>mergeDefault: merge defaults into local resource
mergeDefault-->>diffResource: merged local object
diffResource->>diffPlugins: diff plugins separately
diffPlugins-->>diffResource: pluginsChanged + normalizedPlugins
diffResource->>postprocessSubEvent: adjust sub-event resourceId + inject parentId
postprocessSubEvent-->>diffResource: updated sub-event
diffResource-->>DifferV3: DELETE / CREATE / UPDATE / ONLY_SUB_EVENTS events
end
DifferV3->>DifferV3: unwrap subEvents, sort by order map
DifferV3-->>Caller: Array<ADCSDK.Event>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/differ/src/differv3.ts`:
- Around line 93-102: The differ.logger?.debug call at this location logs
sensitive payloads (local, remote, defaultValue) that may contain auth keys,
OAuth secrets, credentials, and database connection strings without redaction.
Create or use an existing redactor function to sanitize these payloads before
passing them to the logger.debug call, removing or masking any sensitive fields.
Apply the same redaction pattern to all other debug logging calls in the file at
lines 226-229, 415-422, 483-495, and 586-596 where full configs, events, and
plugin configurations are being logged.
- Around line 251-259: The localItem retrieved from localIdMap is a caller-owned
object that should not be mutated by the normalization process. Clone localItem
using cloneDeep (similar to how remoteItem is already cloned at the beginning of
the remote forEach block) before calling unset operations on it to prevent
removing IDs and metadata that will be needed for subsequent syncs. Apply this
same pattern to all locations where localItem is retrieved and normalized,
including the areas around lines 316-323 and 535-540 as noted in the comment.
- Around line 131-147: The diffResource call for ADCSDK.ResourceType.GLOBAL_RULE
is missing plugin default value merging, which causes unchanged local rules to
appear as UPDATE operations when APISIX returns schema defaults. For both the
local and remote global_rules mappings in this diff block, merge the plugin
defaults from defaultValue.plugins[pluginName] into each pluginConfig before
they are passed to diffResource. Apply this same fix to the other similar
global_rules diff locations mentioned at lines 348-353 and 442-449 to ensure
defaults are consistently merged across all global_rule comparisons.
- Around line 263-298: The DELETE event being constructed includes nested
resources in the oldValue property (like routes, stream_routes, upstreams,
consumers, or credentials) which are also being sent as subEvents, causing
inconsistency with UPDATE events and duplicate field transmission. Strip these
nested resource properties from the oldValue when constructing the DELETE event
so that nested fields only appear in subEvents. Apply the same fix to CREATE
events in the location mentioned in the comment (around lines 541-575) where
newValue is set, ensuring both CREATE and DELETE events exclude their nested
resource properties from the parent payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4991379d-8e5c-4d13-a618-f2e669941e66
📒 Files selected for processing (8)
libs/differ/src/differv3.tslibs/differ/src/index.tslibs/differ/src/test/basic.spec.tslibs/differ/src/test/consumer.spec.tslibs/differ/src/test/custom-id.spec.tslibs/differ/src/test/service-upstream.spec.tslibs/differ/src/test/upstream.spec.tslibs/differ/src/test/usecase.spec.ts
Comment on lines
+131
to
+147
| ...differ.diffResource( | ||
| ADCSDK.ResourceType.GLOBAL_RULE, | ||
| Object.entries(local?.global_rules ?? {}).map( | ||
| ([pluginName, pluginConfig]) => [ | ||
| pluginName, | ||
| pluginName, | ||
| pluginConfig as Record<string, unknown>, | ||
| ], | ||
| ) ?? [], | ||
| Object.entries(remote?.global_rules ?? {}).map( | ||
| ([pluginName, pluginConfig]) => [ | ||
| pluginName, | ||
| pluginName, | ||
| pluginConfig as Record<string, unknown>, | ||
| ], | ||
| ) ?? [], | ||
| ), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Merge plugin defaults for GLOBAL_RULE configs.
global_rules are diffed as one resource per plugin name, but they bypass defaultValue.plugins[pluginName]. If APISIX returns schema defaults inside a global-rule plugin config, unchanged local rules can emit UPDATE on every sync.
Proposed direction
- const defaultValue =
- this.defaultValue?.core?.[resourceTypeForDefault] ?? {};
+ const defaultValue =
+ resourceType === ADCSDK.ResourceType.GLOBAL_RULE
+ ? this.defaultValue?.plugins?.[remoteName] ?? {}
+ : this.defaultValue?.core?.[resourceTypeForDefault] ?? {};
const mergedLocalItem = this.mergeDefault(
localItem,
cloneDeep(defaultValue),
);Also applies to: 348-353, 442-449
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/differ/src/differv3.ts` around lines 131 - 147, The diffResource call
for ADCSDK.ResourceType.GLOBAL_RULE is missing plugin default value merging,
which causes unchanged local rules to appear as UPDATE operations when APISIX
returns schema defaults. For both the local and remote global_rules mappings in
this diff block, merge the plugin defaults from defaultValue.plugins[pluginName]
into each pluginConfig before they are passed to diffResource. Apply this same
fix to the other similar global_rules diff locations mentioned at lines 348-353
and 442-449 to ensure defaults are consistently merged across all global_rule
comparisons.
juzhiyuan
approved these changes
Jun 23, 2026
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
2 participants
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.
Description
Since we need to fix #489 and release a patch version for it, we need to prevent any potential implementation errors in v4 from affecting the patch.
Therefore, this PR introduces a feature gate for differ v4. In version 0.27.1, its behavior will be as follows: differ v3 will be enabled by default, and differ v4 will be enabled when the environment variable
ENABLE_DIFFER_V4is set totrueor1. In other words, it is disabled by default and must be explicitly enabled.In future minor releases, this behavior will change to being enabled by default and disabled only when explicitly set. Differ v3 will be completely removed after several releases.
Checklist
Summary by CodeRabbit
New Features
Tests