Skip to content

fix: scope discovery update and delete by namespace (#6827) - #7007

Open
wy471x wants to merge 1 commit into
apache:masterfrom
wy471x:fix_discovery-update-updateSelective-delete-WHERE-id-only
Open

fix: scope discovery update and delete by namespace (#6827)#7007
wy471x wants to merge 1 commit into
apache:masterfrom
wy471x:fix_discovery-update-updateSelective-delete-WHERE-id-only

Conversation

@wy471x

@wy471x wy471x commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

Summary

Changes:

  1. discovery-sqlmap.xml — added AND namespace_id = #{namespaceId, jdbcType=VARCHAR} to the WHERE clauses of update (:217), updateSelective (:248) and delete (:254), so discovery mutations are scoped to the caller's namespace; delete now takes (id, namespaceId) parameters.
  2. DiscoveryMapper.java:134delete(String id) changed to delete(@Param("id") String id, @Param("namespaceId") String namespaceId).
  3. DiscoveryServiceImpl.java:192-206delete accepts namespaceId and verifies the selected DiscoveryDO belongs to the requested namespace before invoking the discovery processor, so cross-namespace ids produce no registry side effects (also fixes a potential NPE when the id does not exist); the mapper delete is then called with the scoped predicate.
  4. SelectorServiceImpl.java:332 / ProxySelectorServiceImpl.java:184 — internal cleanup paths pass discoveryDO.getNamespaceId() (the DO is freshly loaded from DB, so the value is authoritative).
  5. DiscoveryController.javaDELETE /discovery/{discoveryId} now requires a namespaceId request parameter (validated via @Existed(NamespaceMapper)); added @RequiresPermissions("system:plugin:edit") to insertOrUpdate and @RequiresPermissions("system:plugin:delete") to delete, consistent with the Selector/Rule controllers.

Test Cases:

  • DiscoveryMapperTest — H2 integration tests: delete/update/updateSelective with a mismatched namespace mutate 0 rows; with a matching namespace they mutate only the target row and leave the other namespace's row intact.
  • DiscoveryServiceImplTest — delete succeeds in the matching namespace; throws ShenyuException with no processor/mapper side effects on namespace mismatch or when the discovery does not exist.

Verification

  • ./mvnw clean install -Dmaven.javadoc.skip=true passed locally (JDK 21).
  • Targeted tests: 23 run, 0 failures (DiscoveryMapperTest, DiscoveryServiceImplTest, SelectorServiceTest, ProxySelectorServiceTest).
  • checkstyle:check passed.

Note: the dashboard frontend (apache/shenyu-dashboard) currently calls DELETE /discovery/{id} without a namespaceId; a follow-up in that repository is needed to pass the current namespace.

close #6827

related pr: #596

Add namespace_id predicates to the discovery update, updateSelective
and delete statements so that discovery configs can only be modified
within the caller's namespace. DiscoveryService#delete now validates
the namespace of the target discovery before processing, and the
DELETE /discovery/{discoveryId} endpoint requires a namespaceId
parameter and plugin edit/delete permissions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] discovery update/updateSelective/delete WHERE id-only — cross-namespace discovery mutation

1 participant